Beefy Boxes and Bandwidth Generously Provided by pair Networks
No such thing as a small change

comment on









I looked at this source to discover why Perl decimal to binary conversion is not correctly rounded. To cut a long story short, Perl_my_atof2() is a home-brew conversion routine, which makes a mess of some cases.

I wondered why the code doesn't use atof(), and therefore depend on the library writer's skill. There is no commentary to help. I understand, from responses to the bug report I submitted, that the code used to use atof() until its semantics changed in C99 ! Quite a lot of work went into Perl_my_atof2() -- I would have commented on why, at least.

The solution to the problem is to discard all the arithmetic in Perl_my_atof2() and replace it by something which checks that the incoming decimal number string is a valid Perl number, and feed the (possibly modified) string to the real atof(). I would happily bang out a patch to do that... except that it's struggle to work out what the code is expected to do, because there isn't anything at all to say why it is the way it is.

The issue is complicated by the problem of Locale. This is the code that uses Perl_my_atof2() -- I promise you, I have not removed any comments !

NV Perl_my_atof(pTHX_ const char* s) { NV x = 0.0; #ifdef USE_LOCALE_NUMERIC dVAR; if (PL_numeric_local && IN_LOCALE) { NV y; /* Scan the number twice; once using locale and once without +; * choose the larger result (in absolute value). */ Perl_atof2(s, x); SET_NUMERIC_STANDARD(); Perl_atof2(s, y); SET_NUMERIC_LOCAL(); if ((y < 0.0 && y < x) || (y > 0.0 && y > x)) return y; } else Perl_atof2(s, x); #else Perl_atof2(s, x); #endif return x; }
For me there are two issues here:

  1. the key question is, why does it do Perl_my_atof2() twice ?

    Perl_my_atof2() is not a cheap operation. Who knows what it costs to switch the numeric locale. So it looks as though there must be some important semantic requirement here. I would have thought that merited some comment ?

    Suppose Perl_my_atof is given the string '123,456.789' -- if the locale has a ',' decimal point, then the answer is 123point456, otherwise it's 123 (duh). I'm missing something, I expect, but I cannot see why two passes are required or what problem it is supposed to solve. The author must have had to think this through... it would not have cost much to leave some notes for posterity ?

    Perl_my_atof2() itself worries about locale, and will (as far as I can see) accept either the current locale decimal point or '.'. I have a feeling that this means that the two passes in Perl_my_atof() are unnecessary -- if only I could be sure what they were for. If so:

    • I observe that there is no commentary to describe the acceptable form of numbers, so perhaps whoever wrote the two passes didn't fully understand what Perl_my_atof2() does ?

    • or, perhaps more likely, the two passes in Perl_my_atof() were required before Perl_my_atof2() was introduced, but whoever wrote Perl_my_atof2() couldn't see why either, and hence couldn't see that its locale handling is either redundant or makes the two passes redundant.

    A relatively small amount of "why" commentary would help a lot now. Who knows, a little bit of more general commentary might have helped in the past.

  2. sadly, the comment there is, is useless -- it tells you no more than what the code tells you.

Obviously I could settle down and run some test cases, and reverse engineer the requirements. As it happens, I'm not that familiar with the effects of locale switching, which adds to the problem. Just a few comments would do to equip me with the information required to fix the bug...

...further, if I'm right about the two passes in Perl_my_atof being redundant -- a few comments might have helped others too !

In reply to Re^3: Why no comments? by gone2015
in thread Why no comments? by targetsmart

and:  <code> code here </code>

  • Please read these before you post! —
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Log In?

    What's my password?
    Create A New User
    and the web crawler heard nothing...

    How do I use this? | Other CB clients
    Other Users?
    Others taking refuge in the Monastery: (7)
    As of 2019-11-21 08:16 GMT
    Find Nodes?
      Voting Booth?
      Strict and warnings: which comes first?

      Results (104 votes). Check out past polls.