List:Internals« Previous MessageNext Message »
From:Jocelyn Fournier Date:March 26 2007 6:25am
Subject:[PATCH] Patch for Bug#25615, "Status command doesn't report anymore
the Queries per second avg"
View as plain text  
Hi,

Attached a new proposed patch for #25615.

   Jocelyn

Chad MILLER a écrit :
> [add CC jimw]
> 
> On 20 Mar 2007, at 11:32, joce@stripped wrote:
>> I assume implementing %f is linked to WL#2934 ?
> 
> Hi Jocelyn.
> 
> No, I don't think there's a dependent relationship.  It would be nice to 
> have WL#2934 implemented, but clever use of mundane sprintf() could 
> solve this specific case well enough until it is.  We'll want to put a 
> note in that worklog to refactor my_snprintf() later.  We're obviously 
> not using "%f" in many places, and here it's not something that would 
> trigger the platform-specific variances in values.
> 
>> What's the status of the functions in decimal.c ?
>> (especially double2decimal and decimal2string)
> 
> Heh, I didn't know until I just now looked.   double2decimal() uses 
> sprintf(s, "%.16G", from) to get to a string, and then converts that 
> string to the internal decimal type.  That's somewhere we'll eventually 
> refactor when WL#2934 is implemented.  So, if you think you'd like to 
> use that, I imagine it's okay.
> 
> - chad
> 
> 
> 
>>>
>>>> --- sql_parse.cc.old    2007-03-03 17:53:15.000000000 +0100
>>>> +++ sql_parse.cc    2007-03-03 17:53:31.000000000 +0100
>>>> @@ -2119,7 +2119,7 @@
>>>>             &LOCK_status);
>>>>      calc_sum_of_all_status(&current_global_status_var);
>>>>      uptime= (ulong) (thd->start_time - start_time);
>>>> -    length= my_snprintf((char*) buff, buff_len - 1,
>>>> +    length= snprintf((char*) buff, buff_len - 1,
>>>>                          "Uptime: %lu  Threads: %d  Questions: %lu  "
>>>>                          "Slow queries: %lu  Opens: %lu  Flush
>>>> tables: %lu  "
>>>>                          "Open tables: %u  Queries per second avg:
>>>> %.3f",
>>>
>>>
>>> In addition to Sergei's comment that we should never use snprintf()
>>> outside the check for HAVE_SNPRINTF, the return value of snprintf()
>>> varies across implementations, so we should never even trust "length"
>>> in this case.  Thus, another reason we can't accept this approach.
>>> You're right, that adding floating-point decoding to my_snprintf() is
>>> the right approach.
>>>
>>> Jocelyn, are you comfortable implementing "%f"?
>>>
>>> - chad
> 
> 
> -- 
> Chad Miller, Software Developer                         chad@stripped
> MySQL Inc., www.mysql.com
> Orlando, Florida, USA                                13-20z,  UTC-0400
> Office: +1 408 213 6740                         sip:6740@stripped
> 
> 

--- 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
@@ -17,6 +17,7 @@
 #include <m_string.h>
 #include <stdarg.h>
 #include <m_ctype.h>
+#include <decimal.h>
 
 /*
   Limited snprintf() implementations
@@ -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);
+      decimal.buf= (void*) buf1;
+      decimal.len= (int) sizeof(buf1)/sizeof(decimal_digit_t);      
+      larg= va_arg(ap, double);
+      /* converting the double format into decimal format */
+      double2decimal(larg, &decimal);
+      res_length= to_length;
+      /* then converting the decimal into string */ 
+      decimal2string(&decimal, to, 
+                     &res_length, /* max buffer size */
+                                  /* will contain the size of the result */
+                     length ? length : (width ? (decimal.intg + width) : 0), /* precision
*/ 
+                     width, /* digits after the mantis */ 
+                     0);
+      free(buf1);
+      if (res_length > to_length)
+        break;
+      to+= res_length;
+      continue;
+    }
     else if (*fmt == 'c')                       /* Character parameter */
     {
       register int larg;
--- mysql-5.2-ref/unittest/mysys/Makefile.am	2007-03-02 19:24:43.000000000 +0100
+++ mysql-5.2/unittest/mysys/Makefile.am	2007-03-23 01:03:41.000000000 +0100
@@ -17,9 +17,11 @@
 AM_CPPFLAGS     += -I$(top_srcdir)/include -I$(top_srcdir)/unittest/mytap
 
 LDADD 		= $(top_builddir)/unittest/mytap/libmytap.a \
+                  $(top_builddir)/mysys/libmysys.a \
+                  $(top_builddir)/strings/libmystrings.a \
+                  $(top_builddir)/dbug/libdbug.a \
 		  $(top_builddir)/mysys/libmysys.a \
-		  $(top_builddir)/dbug/libdbug.a \
-		  $(top_builddir)/strings/libmystrings.a
+                  $(top_builddir)/strings/libmystrings.a
 
 noinst_PROGRAMS  = bitmap-t base64-t my_atomic-t
 
--- mysql-5.2-ref/libmysql/Makefile.shared	2007-03-02 19:24:28.000000000 +0100
+++ mysql-5.2/libmysql/Makefile.shared	2007-03-22 23:57:59.000000000 +0100
@@ -46,7 +46,7 @@
 			ctype-win1250ch.lo ctype-utf8.lo ctype-extra.lo \
 			ctype-ucs2.lo ctype-gb2312.lo ctype-gbk.lo \
 			ctype-sjis.lo ctype-tis620.lo ctype-ujis.lo \
-			ctype-uca.lo xml.lo my_strtoll10.lo str_alloc.lo 
+			ctype-uca.lo xml.lo my_strtoll10.lo str_alloc.lo decimal.lo
 
 mystringsextra= 	strto.c
 dbugobjects =		dbug.lo # IT IS IN SAFEMALLOC.C sanity.lo
--- mysql-5.2-ref/libmysql/CMakeLists.txt	2007-03-02 19:24:43.000000000 +0100
+++ mysql-5.2/libmysql/CMakeLists.txt	2007-03-22 23:58:47.000000000 +0100
@@ -61,7 +61,7 @@
                      ../strings/strmov.c ../strings/strnlen.c ../strings/strnmov.c
../strings/strtod.c
                      ../strings/strtoll.c ../strings/strtoull.c ../strings/strxmov.c
../strings/strxnmov.c 
                      ../mysys/thr_mutex.c ../mysys/typelib.c ../vio/vio.c
../vio/viosocket.c 
-                     ../vio/viossl.c ../vio/viosslfactories.c ../strings/xml.c)
+                     ../vio/viossl.c ../vio/viosslfactories.c ../strings/xml.c
../strings/decimal.c)
 ADD_DEPENDENCIES(libmysql dbug vio mysys strings GenError zlib yassl taocrypt)
 TARGET_LINK_LIBRARIES(libmysql mysys strings wsock32)
 
--- 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 
 noinst_HEADERS =	cclass.h cname.h regex2.h utils.h engine.c my_regex.h
 libregex_a_SOURCES =	regerror.c regcomp.c regexec.c regfree.c reginit.c
 noinst_PROGRAMS =	re

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