List:Internals« Previous MessageNext Message »
From:Chad MILLER Date:March 20 2007 8:21pm
Subject:Re: review of patch for Bug#25615, "Status command doesn't report anymore the Queries per second avg"
View as plain text  
[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



Attachment: [application/pgp-signature] This is a digitally signed message part PGP.sig
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