List:Internals« Previous MessageNext Message »
From:Sergei Golubchik Date:April 2 2007 1:15pm
Subject:Re: Review of second patch for Bug#25615, "Status command doesn't report anymore the Queries per second avg"
View as plain text  
Hi!

> On 26 Mar 2007, at 02:25, Jocelyn Fournier wrote:
> >--- mysql-5.2-ref/strings/my_vsnprintf.c 2007-03-02 19:24:42.000000000 +0100
> >+++ mysql-5.2/strings/my_vsnprintf.c 2007-03-26 08:01:55.000000000 +0200
> >@@ -152,6 +153,34 @@
> >       to+= res_length;
> >       continue;
> >     }
> >+    else if (*fmt == 'f')   /* Float parameter */
> >+    {
> >+      register double larg;
> >+      uint res_length, to_length;
> >+      to_length= (uint) (end-to);
> >+      decimal_t decimal;
> >+      /* decimal buf and len fields must be set before calling  double2decimal
> */
> >+      /* allocating a buffer of the size of the remaining space in "to" buffer
> */
> >+      char* buf1= malloc(sizeof(char)*to_length);
> 
> Use my_malloc() and my_free(), always.

my_alloca()/my_afree() would be even more appropriate here.
 
> Our test suite runs the programs in the "unittest" directory, but  
> since I spend most of my time in the 5.0 tree, I don't offhand know  
> how.

'make test-unit' does it. 'make test' includes 'make test-unit'.

> You may need to add a reference to it somewhere.

It's enough to have a binary matching *-t somewhere where unittest looks for it.
 
> >--- mysql-5.2-ref/regex/Makefile.am	2007-03-02 19:24:43.000000000  +0100
> >+++ mysql-5.2/regex/Makefile.am	2007-03-23 00:47:50.000000000 +0100
> >@@ -17,7 +17,7 @@
> >
> > INCLUDES =		-I$(top_builddir)/include -I$(top_srcdir)/include
> > noinst_LIBRARIES =	libregex.a
> >-LDADD=			libregex.a $(top_builddir)/strings/libmystrings.a
> >+LDADD=			libregex.a $(top_builddir)/strings/libmystrings.a
> $(top_builddir)/dbug/libdbug.a $(top_builddir)/mysys/libmysys.a
> $(top_builddir)/strings/libmystrings.a

I'm not sure it's a good idea to have libmystrings.a depending on
libmysys.a. It adds a circular dependency, as you can see.

What caused it ? malloc() ?

Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Senior Software Developer
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Radlkoferstr. 2, D-81373 München
       <___/  Geschäftsführer: Hans von Bell, Kaj Arnö - HRB
München 162140
Thread
review of patch for Bug#25615, "Status command doesn't report anymore the Queries per second avg"Chad MILLER20 Mar
  • Re: review of patch for Bug#25615,"Status command doesn't report anymore the Queries per second avg"joce20 Mar
    • Re: review of patch for Bug#25615, "Status command doesn't report anymore the Queries per second avg"Chad MILLER20 Mar
      • Re: review of patch for Bug#25615, "Status command doesn't reportanymore the Queries per second avg"Jocelyn Fournier25 Mar
        • Re: debugger suggestionsChad MILLER26 Mar
          • Re: debugger suggestionsjoce26 Mar
      • [PATCH] Patch for Bug#25615, "Status command doesn't report anymorethe Queries per second avg"Jocelyn Fournier26 Mar
        • Review of second patch for Bug#25615, "Status command doesn't report anymore the Queries per second avg"Chad MILLER2 Apr
          • Re: Review of second patch for Bug#25615, "Status command doesn'treport anymore the Queries per second avg"Baron Schwartz2 Apr
            • Re: Review of second patch for Bug#25615, "Status command doesn't report anymore the Queries per second avg"Chad MILLER2 Apr
          • Re: Review of second patch for Bug#25615,joce2 Apr
          • Re: Review of second patch for Bug#25615, "Status command doesn't report anymore the Queries per second avg"Sergei Golubchik2 Apr
            • Re: Review of second patch for Bug#25615, "Status command doesn'treport anymore the Queries per second avg"joce@presence-pc.com8 Apr
              • Re: Review of second patch for Bug#25615, "Status command doesn't report anymore the Queries per second avg"Sergei Golubchik9 Apr
          • Re: Review of second patch for Bug#25615, "Status command doesn'treport anymore the Queries per second avg"joce@presence-pc.com9 Apr
            • Re: Review of second patch for Bug#25615, "Status command doesn'treport anymore the Queries per second avg"joce@presence-pc.com9 Apr
              • Re: Review of second patch for Bug#25615, "Status command doesn't report anymore the Queries per second avg"Sergei Golubchik9 Apr
              • Re: Review of second patch for Bug#25615, "Status command doesn'treport anymore the Queries per second avg"joce@presence-pc.com9 Apr
                • Re: Review of second patch for Bug#25615, "Status command doesn'treport anymore the Queries per second avg"joce@presence-pc.com9 Apr
                  • Review of fourth patch for Bug#25615, "Status command doesn't report anymore the Queries per second avg"Chad MILLER10 Apr
                    • Re: Review of fourth patch for Bug#25615, "Status command doesn'treport anymore the Queries per second avg"Jocelyn Fournier10 Apr