Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #59511 > unrolled thread

strtoll discussion

Back to article view | Back to comp.lang.c

Contents

  strtoll discussion jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-03 14:28 +0200
    Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-03 14:46 +0100
    Re: strtoll discussion Keith Thompson <kst-u@mib.org> - 2015-04-03 11:08 -0700
    Re: strtoll discussion James Kuyper <jameskuyper@verizon.net> - 2015-04-03 15:23 -0400
      Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-03 20:55 +0100
        Re: strtoll discussion glen herrmannsfeldt <gah@ugcs.caltech.edu> - 2015-04-03 20:41 +0000
          Re: strtoll discussion Malcolm McLean <malcolm.mclean5@btinternet.com> - 2015-04-03 13:52 -0700
            Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-03 22:25 +0100
              Re: strtoll discussion Bartc <bc@freeuk.com> - 2015-04-04 00:17 +0100
                Re: strtoll discussion Malcolm McLean <malcolm.mclean5@btinternet.com> - 2015-04-03 16:27 -0700
              Re: strtoll discussion Ben Bacarisse <ben.usenet@bsb.me.uk> - 2015-04-04 02:07 +0100
                Re: strtoll discussion Malcolm McLean <malcolm.mclean5@btinternet.com> - 2015-04-04 03:44 -0700
                  Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-04 12:13 +0100
      Re: strtoll discussion James Kuyper <jameskuyper@verizon.net> - 2015-04-03 16:11 -0400
      Re: strtoll discussion Keith Thompson <kst-u@mib.org> - 2015-04-03 14:12 -0700
      Re: strtoll discussion jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-03 23:56 +0200
    Re: strtoll discussion Ian Collins <ian-news@hotmail.com> - 2015-04-04 08:41 +1300
      Re: strtoll discussion jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-03 23:33 +0200
    Re: strtoll discussion James Lothian <james@firstnamelastname.freeserve.co.uk> - 2015-04-03 20:51 +0100
      Re: strtoll discussion asetofsymbols@gmail.com - 2015-04-03 13:09 -0700
        Re: strtoll discussion asetofsymbols@gmail.com - 2015-04-03 13:16 -0700
        Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-03 21:19 +0100
          Re: strtoll discussion asetofsymbols@gmail.com - 2015-04-03 13:28 -0700
            Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-03 22:29 +0100
            Re: strtoll discussion jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-03 23:33 +0200
        Re: strtoll discussion jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-03 23:34 +0200
          Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-03 22:42 +0100
            Re: strtoll discussion Keith Thompson <kst-u@mib.org> - 2015-04-03 15:08 -0700
            Re: strtoll discussion jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-04 00:13 +0200
              Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-03 23:44 +0100
                Re: strtoll discussion jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-04 00:48 +0200
                  Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-04 00:03 +0100
                  Re: strtoll discussion Lew Pitcher <lew.pitcher@digitalfreehold.ca> - 2015-04-03 19:31 -0400
                    Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-04 00:37 +0100
                      Re: strtoll discussion Lew Pitcher <lew.pitcher@digitalfreehold.ca> - 2015-04-04 10:29 -0400
                        Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-04 15:54 +0100
                        Re: strtoll discussion Malcolm McLean <malcolm.mclean5@btinternet.com> - 2015-04-04 08:07 -0700
                          Re: strtoll discussion Bartc <bc@freeuk.com> - 2015-04-04 16:28 +0100
                            Re: strtoll discussion Malcolm McLean <malcolm.mclean5@btinternet.com> - 2015-04-04 08:57 -0700
                              Re: strtoll discussion Bartc <bc@freeuk.com> - 2015-04-04 17:03 +0100
                                Re: strtoll discussion Malcolm McLean <malcolm.mclean5@btinternet.com> - 2015-04-04 09:44 -0700
                                  Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-04 18:05 +0100
                                    Re: strtoll discussion Malcolm McLean <malcolm.mclean5@btinternet.com> - 2015-04-04 23:32 -0700
                                      Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-05 11:00 +0100
                                        Re: strtoll discussion Malcolm McLean <malcolm.mclean5@btinternet.com> - 2015-04-05 13:18 -0700
                                          Re: strtoll discussion Richard Damon <Richard@Damon-Family.org> - 2015-04-05 17:37 -0400
                                            Re: strtoll discussion Malcolm McLean <malcolm.mclean5@btinternet.com> - 2015-04-05 15:00 -0700
                                              Re: strtoll discussion Robert Wessel <robertwessel2@yahoo.com> - 2015-04-05 22:17 -0500
                            Re: strtoll discussion Richard Damon <Richard@Damon-Family.org> - 2015-04-04 18:54 -0400
                              Re: strtoll discussion Bartc <bc@freeuk.com> - 2015-04-05 12:14 +0100
                    Re: strtoll discussion glen herrmannsfeldt <gah@ugcs.caltech.edu> - 2015-04-04 00:19 +0000
                  Re: strtoll discussion Keith Thompson <kst-u@mib.org> - 2015-04-03 17:48 -0700
                  Re: strtoll discussion James Kuyper <jameskuyper@verizon.net> - 2015-04-08 11:23 -0400
                    Re: strtoll discussion jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-08 19:35 +0200
                    Re: strtoll discussion Keith Thompson <kst-u@mib.org> - 2015-04-08 10:47 -0700
                      Re: strtoll discussion James Kuyper <jameskuyper@verizon.net> - 2015-04-08 14:22 -0400
    lltostr/ulltostr jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-04 13:56 +0200
      Re: lltostr/ulltostr Richard Heathfield <rjh@cpax.org.uk> - 2015-04-04 13:36 +0100
      Re: lltostr/ulltostr Barry Schwarz <schwarzb@dqel.com> - 2015-04-04 18:54 -0700
        Re: lltostr/ulltostr Ian Collins <ian-news@hotmail.com> - 2015-04-05 14:29 +1200
      Re: lltostr/ulltostr Ralf Damaschke <rwspam@gmx.de> - 2015-04-05 16:58 +0000
        Re: lltostr/ulltostr jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-05 20:39 +0200
          Re: lltostr/ulltostr glen herrmannsfeldt <gah@ugcs.caltech.edu> - 2015-04-05 20:19 +0000
        Re: lltostr/ulltostr Keith Thompson <kst-u@mib.org> - 2015-04-05 13:18 -0700
          Re: lltostr/ulltostr Ralf Damaschke <rwspam@gmx.de> - 2015-04-06 14:28 +0000
            Re: lltostr/ulltostr Keith Thompson <kst-u@mib.org> - 2015-04-06 08:40 -0700
    Re: strtoll discussion Tim Rentsch <txr@alumni.caltech.edu> - 2015-04-04 13:04 -0700
      Re: strtoll discussion jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-04 22:52 +0200
        Re: strtoll discussion Richard Heathfield <rjh@cpax.org.uk> - 2015-04-04 22:14 +0100
        Re: strtoll discussion Ben Bacarisse <ben.usenet@bsb.me.uk> - 2015-04-05 00:24 +0100
        Re: strtoll discussion Geoff <geoff@invalid.invalid> - 2015-04-04 16:51 -0700
        Re: strtoll discussion jacobnavia <jacob@jacob.remcomp.fr> - 2015-04-05 23:32 +0200
          Re: strtoll discussion Tim Rentsch <txr@alumni.caltech.edu> - 2015-04-16 09:21 -0700
        Re: strtoll discussion Tim Rentsch <txr@alumni.caltech.edu> - 2015-04-17 02:21 -0700
      Re: strtoll discussion Keith Thompson <kst-u@mib.org> - 2015-04-04 15:36 -0700
        Re: strtoll discussion Tim Rentsch <txr@alumni.caltech.edu> - 2015-04-16 09:19 -0700
          Re: strtoll discussion Keith Thompson <kst-u@mib.org> - 2015-04-16 10:43 -0700
            Re: strtoll discussion Tim Rentsch <txr@alumni.caltech.edu> - 2015-04-17 07:59 -0700
    Re: strtoll discussion Philipp Klaus Krause <pkk@spth.de> - 2015-04-07 11:46 +0200

Page 4 of 4 — ← Prev page 1 2 3 [4]

#59634 — Re: lltostr/ulltostr

jacobnavia schrieb:

> int ulltostr(unsigned long long number,
>               char *outStr,size_t strSize, int base)

Did I miss something? A joke, a forgery?
The function obviously fails for each number greater than LLONG_MAX
unless you feel that NUUTTPU0TQORSPPRPNN is an appropriate decimal
representation of LLONG_MAX+10ULL (assuming 64 bit).

[toc] | [prev] | [next] | [standalone]

#59647 — Re: lltostr/ulltostr

Le 05/04/2015 18:58, Ralf Damaschke a écrit :
> jacobnavia schrieb:
>
>> int ulltostr(unsigned long long number,
>>                char *outStr,size_t strSize, int base)
>
> Did I miss something? A joke, a forgery?
> The function obviously fails for each number greater than LLONG_MAX
> unless you feel that NUUTTPU0TQORSPPRPNN is an appropriate decimal
> representation of LLONG_MAX+10ULL (assuming 64 bit).
>

The problem is that lldiv makes only a SIGNED division.

Since lldiv speeds up things compared to the code I had before (mod and 
division, two operations) I replaced

m = number % base;
number = number/ base;

with:
lldiv_t d =lldiv(number, base);
m = d.rem;
number = d.quot;

Without thinking about the signed/unsigned problem...

Thanks for your input.

[toc] | [prev] | [next] | [standalone]

#59655 — Re: lltostr/ulltostr

jacobnavia <jacob@jacob.remcomp.fr> wrote:
> Le 05/04/2015 18:58, Ralf Damaschke a écrit :
>> jacobnavia schrieb:

>>> int ulltostr(unsigned long long number,
>>>                char *outStr,size_t strSize, int base)

>> Did I miss something? A joke, a forgery?
>> The function obviously fails for each number greater than LLONG_MAX
>> unless you feel that NUUTTPU0TQORSPPRPNN is an appropriate decimal
>> representation of LLONG_MAX+10ULL (assuming 64 bit).
 
> The problem is that lldiv makes only a SIGNED division.
 
> Since lldiv speeds up things compared to the code I had before (mod and 
> division, two operations) I replaced

There is a paper on how to do unsigned divide if you only have signed
divide hardware from many years ago. 

-- glen

[toc] | [prev] | [next] | [standalone]

#59654 — Re: lltostr/ulltostr

Ralf Damaschke <rwspam@gmx.de> writes:
> jacobnavia schrieb:
>> int ulltostr(unsigned long long number,
>>               char *outStr,size_t strSize, int base)
>
> Did I miss something? A joke, a forgery?

It's possible to post a bug report without insults.

> The function obviously fails for each number greater than LLONG_MAX
> unless you feel that NUUTTPU0TQORSPPRPNN is an appropriate decimal
> representation of LLONG_MAX+10ULL (assuming 64 bit).

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#59689 — Re: lltostr/ulltostr

Keith Thompson schrieb:

> Ralf Damaschke <rwspam@gmx.de> writes:

>> Did I miss something? A joke, a forgery?
> 
> It's possible to post a bug report without insults.

Yes, it is. I didn't meant to insult anyone and I still don't see
any insult in what I have written.

Being considerably surprised I even compiled and ran the function
to check whether some coincidence will give the right result anyway.
Meanwhile Jacob gave a plausible reason for this bug.

[toc] | [prev] | [next] | [standalone]

#59692 — Re: lltostr/ulltostr

Ralf Damaschke <rwspam@gmx.de> writes:
> Keith Thompson schrieb:
>> Ralf Damaschke <rwspam@gmx.de> writes:
>>> Did I miss something? A joke, a forgery?
>> 
>> It's possible to post a bug report without insults.
>
> Yes, it is. I didn't meant to insult anyone and I still don't see
> any insult in what I have written.

Seeing a bug and concluding that the code was likely posted as a joke
could easily be taken as an insult.  I'll take your word that that
wasn't your intent.

[...]

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#59596

jacobnavia <jacob@jacob.remcomp.fr> writes:

> I have written a document about the discusssion that we had in
> this group.  It would be nice to give to newcomers here, so that
> they can read non-trivial code that is well documented.  [snip]
>
> The URL is:  http://www.cs.virginia.edu/~lcc-win32/strtoll.pdf

I have a question on the text, and several comments on
the code.

In the text, you gave no mention of the method I described
for preventing overflow.  Is that because you decided not to
include it, or did you not see the posting?

For the code, here are several comments and corrections.

On page 7, the return types of strtol() and strtoul() are
reversed.  Also the names of the flags are a little bit
misleading, since [un]signed long may not be 32 bits.

On page 8, I recommend moving line 37, ie,

    if ( endptr ) *endptr = (char *) input;

after lines 49-53, ie, after

    if ( base < 0 || base == 1 || base > 36
       || input == NULL ) {
	errno = ERANGE ;
	return 0;
    }

which is more in keeping (AIUI) with POSIX and with other
implementations.  Related to that, the value ERANGE for errno on
line 51 is not really the best choice here, since the problem is
not a range error but a domain error.  Using EINVAL is a better
match to the actual circumstance, and also is more in keeping
(again AIUI) with POSIX and with other implementations.

On page 9, for lines 62-66, ie, this text

    if (*input == '0') {
	input++; /* Swallow the zero */
	ReadSomething = 1;
	if( (base == 0 || base == 16) &&
	   (input[1] == 'X') || (input[1] == 'x')) {

the condition in the second 'if' has several bugs.  First the
indexing of 'input' should be 'index[0]', not 'index[1]'.
Second the second line is mis-parenthesized -- the two
parentheses closest to the '||' should be removed.  Third there
needs to be another condition added, to be sure the digit (if
any) following the 'x' or 'X' is a hexadecimal digit.  Putting
these together the second 'if' test should be more like this:

	if( (base == 0 || base == 16) &&
	   (input[0] == 'X' || input[0] == 'x') &&
           is_hex_digit( input[1] )
        ) {

so that inputs starting '0x' but without a following hexadecimal
digit are recognized correctly, that is, as just '0' and nothing
else.  (The function is_hex_digit() does what its name implies.)

On page 10, line 83, there is this assignment

    nbmax = (char)((char)'0' + (char)(base - 1));

All of the casts here are unnecessary.  In fact nearly all of the
casts in the StrtoXXX() function are unnecessary;  the only
exceptions are the two (char*) casts in assigning to 'endptr'.

On line 87, ie, the assignment

    cbmaxLower = (char)('a' + (base - 11));

(which also has a superfluous cast), the code is operating on the
tacit assumption that lower case letters are contiguous (and
increasing).  The assumption is okay but I suggest it deserves
a comment or a test of some sort (such as assert()).  So at least
this:

    cbmaxLower = 'a' + (base - 11);
    // test:  cbmaxLower == "0123456789abcdefghijklmnopqrstuvwxyz"[ base-1 ]

which both states the test and also serves to convey just what
relationship is being calculated.  There is an analogous change
for line 90.

On lines 97-98, ie, these two assignments

n    maxval = LLONG_MAX ;
    errval = (unsigned long long) LLONG_MIN;

the cast is again superfluous, but more importantly the setting
of 'maxval' is wrong.  These lines may be correctly written as

    maxval = -(LLONG_MIN+1) + 1ULL;
    errval = LLONG_MIN;

A similar change is needed for line 107, ie, this line

    errval = maxval = LONG_MAX;

should be revised to, for example, these lines

    if ( sign == '-' ) {
        maxval = -(LONG_MIN+1) + 1ULL;
        errval = LONG_MIN;
    } else  errval = maxval = LONG_MAX;

On page 11, the final 'else' clause (lines 164-169):

    else if ( sign == '-' ) {
        if ( Flags & (SIGNED32 | UNSIGNED32) )
            value = - (long long) (value & 0xffffffff);
        else
            value = - (long long) value;
    }

has unnecessary casts, and also a dependency on an assumption
that 'long' is only 32 bits.  In fact all these complications are
not necessary.  These lines may be correctly written more simply
as

    else if ( sign == '-' ) {
        value = -value;
    }

which should produce correct results on your implementation (it
did in my tests).  Strictly speaking this result depends on an
implementation-defined behavior (in the two calling functions
strtol() and strtoll()), converting an unsigned long long to a
long or long long, respectively.  It is reasonable to expect this
conversion will do what we want, but it seems a good idea to put
in at least a comment to that effect.  If the above changes are
made the only places where this implementation-defined behavior
is relevant is in strtol() and strtoll().

[toc] | [prev] | [next] | [standalone]

#59597

Le 04/04/2015 22:04, Tim Rentsch a écrit :
> jacobnavia <jacob@jacob.remcomp.fr> writes:
>
>> I have written a document about the discusssion that we had in
>> this group.  It would be nice to give to newcomers here, so that
>> they can read non-trivial code that is well documented.  [snip]
>>
>> The URL is:  http://www.cs.virginia.edu/~lcc-win32/strtoll.pdf
>
> I have a question on the text, and several comments on
> the code.
>
> In the text, you gave no mention of the method I described
> for preventing overflow.  Is that because you decided not to
> include it, or did you not see the posting?
>

Sorry, could you send it to my email address? or here is the same.
Thanks

> For the code, here are several comments and corrections.
>
> On page 7, the return types of strtol() and strtoul() are
> reversed.  Also the names of the flags are a little bit
> misleading, since [un]signed long may not be 32 bits.
>
> On page 8, I recommend moving line 37, ie,
>
>      if ( endptr ) *endptr = (char *) input;
>
> after lines 49-53, ie, after
>
>      if ( base < 0 || base == 1 || base > 36
>         || input == NULL ) {
> 	errno = ERANGE ;
> 	return 0;
>      }
>

OK, no problems with that

> which is more in keeping (AIUI) with POSIX and with other
> implementations.  Related to that, the value ERANGE for errno on
> line 51 is not really the best choice here, since the problem is
> not a range error but a domain error.  Using EINVAL is a better
> match to the actual circumstance, and also is more in keeping
> (again AIUI) with POSIX and with other implementations.
>
> On page 9, for lines 62-66, ie, this text
>
>      if (*input == '0') {
> 	input++; /* Swallow the zero */
> 	ReadSomething = 1;
> 	if( (base == 0 || base == 16) &&
> 	   (input[1] == 'X') || (input[1] == 'x')) {
>
> the condition in the second 'if' has several bugs.  First the
> indexing of 'input' should be 'index[0]', not 'index[1]'.

Thanks, I introduced this bug when I factored in the input++ without 
adjusting the indexes!

Thanks for your help

> Second the second line is mis-parenthesized -- the two
> parentheses closest to the '||' should be removed.

Yes, that it would be better!

  Third there
> needs to be another condition added, to be sure the digit (if
> any) following the 'x' or 'X' is a hexadecimal digit.  Putting
> these together the second 'if' test should be more like this:
>
> 	if( (base == 0 || base == 16) &&
> 	   (input[0] == 'X' || input[0] == 'x') &&
>             is_hex_digit( input[1] )
>          ) {
>
> so that inputs starting '0x' but without a following hexadecimal
> digit are recognized correctly, that is, as just '0' and nothing
> else.

This happens already since the first digit will be 'x' in base octal and 
that would stop the conversion returning zero.

If I am correct, now I am unsure after all those bugs I did not see :-)


(The function is_hex_digit() does what its name implies.)
>
> On page 10, line 83, there is this assignment
>
>      nbmax = (char)((char)'0' + (char)(base - 1));
>
> All of the casts here are unnecessary.

Yes, but you should convince MSVC not to emit a warning at those because 
you are automatically casting an int into a char and that is narrowing, 
what could lead to data loss. MSVC will only stop emitting that warning 
if there is a cast.

I hink I should add a comment explicitely saying this. Thanks


In fact nearly all of the
> casts in the StrtoXXX() function are unnecessary;  the only
> exceptions are the two (char*) casts in assigning to 'endptr'.
>
> On line 87, ie, the assignment
>
>      cbmaxLower = (char)('a' + (base - 11));
>
> (which also has a superfluous cast), the code is operating on the
> tacit assumption that lower case letters are contiguous (and
> increasing).  The assumption is okay but I suggest it deserves
> a comment or a test of some sort (such as assert()).  So at least
> this:
>
>      cbmaxLower = 'a' + (base - 11);
>      // test:  cbmaxLower == "0123456789abcdefghijklmnopqrstuvwxyz"[ base-1 ]
>
> which both states the test and also serves to convey just what
> relationship is being calculated.  There is an analogous change
> for line 90.
>


OK I wil add your comments.

> On lines 97-98, ie, these two assignments
>
>      maxval = LLONG_MAX ;
>      errval = (unsigned long long) LLONG_MIN;
>
> the cast is again superfluous, but more importantly the setting
> of 'maxval' is wrong.  These lines may be correctly written as
>
>      maxval = -(LLONG_MIN+1) + 1ULL;
>      errval = LLONG_MIN;
>

I do not follow you here. Why those additions, note that maxval is 
unsigned...


> A similar change is needed for line 107, ie, this line
>
>      errval = maxval = LONG_MAX;
>
> should be revised to, for example, these lines
>
>      if ( sign == '-' ) {
>          maxval = -(LONG_MIN+1) + 1ULL;
>          errval = LONG_MIN;
>      } else  errval = maxval = LONG_MAX;
>
> On page 11, the final 'else' clause (lines 164-169):
>
>      else if ( sign == '-' ) {
>          if ( Flags & (SIGNED32 | UNSIGNED32) )
>              value = - (long long) (value & 0xffffffff);
>          else
>              value = - (long long) value;
>      }
>
> has unnecessary casts, and also a dependency on an assumption
> that 'long' is only 32 bits.  In fact all these complications are
> not necessary.  These lines may be correctly written more simply
> as
>
>      else if ( sign == '-' ) {
>          value = -value;
>      }
>
> which should produce correct results on your implementation (it
> did in my tests).  Strictly speaking this result depends on an
> implementation-defined behavior (in the two calling functions
> strtol() and strtoll()), converting an unsigned long long to a
> long or long long, respectively.  It is reasonable to expect this
> conversion will do what we want, but it seems a good idea to put
> in at least a comment to that effect.  If the above changes are
> made the only places where this implementation-defined behavior
> is relevant is in strtol() and strtoll().
>

Well that was REALLY helpful. I will add your changes tomorrow.

[toc] | [prev] | [next] | [standalone]

#59599

On 04/04/15 21:52, jacobnavia wrote:
> Le 04/04/2015 22:04, Tim Rentsch a écrit :


<snip>

>>
>> On page 10, line 83, there is this assignment
>>
>>      nbmax = (char)((char)'0' + (char)(base - 1));
>>
>> All of the casts here are unnecessary.
>
> Yes, but you should convince MSVC not to emit a warning at those because
> you are automatically casting an int into a char and that is narrowing,
> what could lead to data loss. MSVC will only stop emitting that warning
> if there is a cast.
>
> I hink I should add a comment explicitely saying this. Thanks
>

In my view, the purpose of warnings is to draw your attention to 
possible problems, so that you can check the quality of the code and 
improve it if need be. Adding casts almost never improves the quality of 
the code. If the net result of a warning is to /reduce/ the quality of 
the code, then it has defeated its own purpose.

When a diagnostic message draws your attention to a potential problem 
that, in fact, you know not to be a problem, there is a case for adding 
a comment to the code explaining why the diagnostic message is generated 
and why, in this case, after taking due note of it, you have decided to 
leave the code as it is. For example:

/* base is in the range 2-36. Subtracting one gives us the maximum
    digit in the base, 1-35. The only circumstance in which adding
    '0' to this will result in a value > CHAR_MAX is that the character
    set has '0' in the upper 34 code points of the character set.
    This is not true for ASCII. It /is/ true for EBCDIC, but this
    routine is targeted only at ASCII-based implementations. */

If you must cast (and there is no particular reason why you should), 
what is wrong with:

nbmax = (char)('0' + base - 1);

? This would at least remove two of the three casts.

<snip>

-- 
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within

[toc] | [prev] | [next] | [standalone]

#59603

jacobnavia <jacob@jacob.remcomp.fr> writes:

> Le 04/04/2015 22:04, Tim Rentsch a écrit :
<snip>
>> On page 10, line 83, there is this assignment
>>
>>      nbmax = (char)((char)'0' + (char)(base - 1));
>>
>> All of the casts here are unnecessary.
>
> Yes, but you should convince MSVC not to emit a warning at those
> because you are automatically casting an int into a char and that is
> narrowing, what could lead to data loss. MSVC will only stop emitting
> that warning if there is a cast.

Or you could store the int result in an int.  I haven't looked at this
code in context, but from what's shown here there does not seem to be
any reason to convert the int result to char.

<snip>
-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#59604

On Sat, 04 Apr 2015 22:52:50 +0200, jacobnavia
<jacob@jacob.remcomp.fr> wrote:
Le 04/04/2015 22:04, Tim Rentsch a écrit :
[snip]
>> On page 10, line 83, there is this assignment
>>
>>      nbmax = (char)((char)'0' + (char)(base - 1));
>>
>> All of the casts here are unnecessary.
>
>Yes, but you should convince MSVC not to emit a warning at those because 
>you are automatically casting an int into a char and that is narrowing, 
>what could lead to data loss. MSVC will only stop emitting that warning 
>if there is a cast.
>
>I hink I should add a comment explicitely saying this. Thanks

While I understand nbmax will always be a small integer value, what do
you gain by typing it as char and not int and thereby avoid the
warnings and casts altogether?

[toc] | [prev] | [next] | [standalone]

#59660

Le 04/04/2015 22:52, jacobnavia a écrit :
> Sorry, could you send it to my email address? or here is the same.
> Thanks

I found it. Do not worry

[toc] | [prev] | [next] | [standalone]

#60151

jacobnavia <jacob@jacob.remcomp.fr> writes:

> Le 04/04/2015 22:52, jacobnavia a ecrit :
>> Sorry, could you send it to my email address?  or here is the same.
>
> I found it.  Do not worry

Okay.  I sent a test message to your email address in
case you might be interested in some further comments.
Did you get that?

[toc] | [prev] | [next] | [standalone]

#60213

jacobnavia <jacob@jacob.remcomp.fr> writes:

> Le 04/04/2015 22:04, Tim Rentsch a ecrit :
>> jacobnavia <jacob@jacob.remcomp.fr> writes:
>> [snip]
>>
>> On page 9, for lines 62-66, ie, this text
>>
>>      if (*input == '0') {
>> 	input++; /* Swallow the zero */
>> 	ReadSomething = 1;
>> 	if( (base == 0 || base == 16) &&
>> 	   (input[1] == 'X') || (input[1] == 'x')) {
>>
>> the condition in the second 'if' has several bugs.  First the
>> indexing of 'input' should be 'index[0]', not 'index[1]'.
>
> Thanks, I introduced this bug when I factored in the input++
> without adjusting the indexes!
>
> Thanks for your help
>
>> Second the second line is mis-parenthesized -- the two
>> parentheses closest to the '||' should be removed.
>
> Yes, that it would be better!
>
>> Third there
>> needs to be another condition added, to be sure the digit (if
>> any) following the 'x' or 'X' is a hexadecimal digit.  Putting
>> these together the second 'if' test should be more like this:
>>
>> 	if( (base == 0 || base == 16) &&
>> 	   (input[0] == 'X' || input[0] == 'x') &&
>>             is_hex_digit( input[1] )
>>          ) {
>>
>> so that inputs starting '0x' but without a following hexadecimal
>> digit are recognized correctly, that is, as just '0' and nothing
>> else.
>
> This happens already since the first digit will be 'x' in base
> octal and that would stop the conversion returning zero.
>
> If I am correct, now I am unsure after all those bugs I did not
> see :-)

Do take a look - I think you will find that the test for a
following hex digit is needed here to get a result consistent
with what the Standard specifies.

> Well that was REALLY helpful.  I will add your changes tomorrow.

You are most welcome sir.  I have some additional comments
if you are interested, you could post a followup here or
send email.

[toc] | [prev] | [next] | [standalone]

#59600

Tim Rentsch <txr@alumni.caltech.edu> writes:
[...]
>                   Related to that, the value ERANGE for errno on
> line 51 is not really the best choice here, since the problem is
> not a range error but a domain error.  Using EINVAL is a better
> match to the actual circumstance, and also is more in keeping
> (again AIUI) with POSIX and with other implementations.

But EINVAL is not one of the 3 errno values defined in the standard for
<errno.h>, so you can't portably assume that it exists.  (The portable
values are EDOM, EILSEQ, and ERANGE; EDOM is used only in <math.h>.)

[...]

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#60149

Keith Thompson <kst-u@mib.org> writes:

> Tim Rentsch <txr@alumni.caltech.edu> writes:
> [...]
>>                   Related to that, the value ERANGE for errno on
>> line 51 is not really the best choice here, since the problem is
>> not a range error but a domain error.  Using EINVAL is a better
>> match to the actual circumstance, and also is more in keeping
>> (again AIUI) with POSIX and with other implementations.
>
> But EINVAL is not one of the 3 errno values defined in the standard for
> <errno.h>, so you can't portably assume that it exists.  (The portable
> values are EDOM, EILSEQ, and ERANGE;  EDOM is used only in <math.h>.)

Right, I should have mentioned that.  I would hope that the
environment for his implementation includes EINVAL, but to
protect against the eventuality that it does not the error
code used can be based on a preprocessor test, eg,

    #include <errno.h>

    #ifdef EINVAL
    #  define  BAD_ARGUMENT  EINVAL
    #else
    #  define  BAD_ARGUMENT  EDOM
    #endif

    /* Then use 'BAD_ARGUMENT' to set 'errno' */

Alternatively the defined-ness of EINVAL can be tested and if it
is not present then a #error used to end the compilation.  There
is value in punishing implementations that don't have at least
minimal support for Posix.  :)

Incidentally, I see nothing wrong with re-using EDOM for this
purpose in the case when there is no EINVAL (and EDOM is a
better choice than ERANGE).

[toc] | [prev] | [next] | [standalone]

#60161

Tim Rentsch <txr@alumni.caltech.edu> writes:
> Keith Thompson <kst-u@mib.org> writes:
>> Tim Rentsch <txr@alumni.caltech.edu> writes:
>> [...]
>>>                   Related to that, the value ERANGE for errno on
>>> line 51 is not really the best choice here, since the problem is
>>> not a range error but a domain error.  Using EINVAL is a better
>>> match to the actual circumstance, and also is more in keeping
>>> (again AIUI) with POSIX and with other implementations.
>>
>> But EINVAL is not one of the 3 errno values defined in the standard for
>> <errno.h>, so you can't portably assume that it exists.  (The portable
>> values are EDOM, EILSEQ, and ERANGE;  EDOM is used only in <math.h>.)
>
> Right, I should have mentioned that.  I would hope that the
> environment for his implementation includes EINVAL, but to
> protect against the eventuality that it does not the error
> code used can be based on a preprocessor test, eg,
>
>     #include <errno.h>
>
>     #ifdef EINVAL
>     #  define  BAD_ARGUMENT  EINVAL
>     #else
>     #  define  BAD_ARGUMENT  EDOM
>     #endif
>
>     /* Then use 'BAD_ARGUMENT' to set 'errno' */
>
> Alternatively the defined-ness of EINVAL can be tested and if it
> is not present then a #error used to end the compilation.  There
> is value in punishing implementations that don't have at least
> minimal support for Posix.  :)

I thought about that, but it means calling code potentially has to check
for two different errno values.  The benefits of using a more sensible
value on systems where it exists is outweighed, IMHO, by the benefits of
consistency.

If we want to make the C standard depend on the POSIX standard, then I
suppose that could be done -- but I don't think it would fly.

> Incidentally, I see nothing wrong with re-using EDOM for this
> purpose in the case when there is no EINVAL (and EDOM is a
> better choice than ERANGE).

Agreed.  Setting errno to EDOM in a case where the standard doesn't
define the behavior is (a) conforming and (b) IMHO reasonable.

The current discussion is about how to implement strtoll as specified
by the current standard.

But I wouldn't mind seeing the standard updated to (a) define the
behavior of stroll for an invalid base (I think that's the case we're
talking about) and perhaps (b) to add EINVAL as one of the standard
errno values.

On the other hand, I strongly suspect that the value passed for base is
almost always a constant, so defining the behavior for an incorrect
value isn't a huge priority.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#60235

Keith Thompson <kst-u@mib.org> writes:

> Tim Rentsch <txr@alumni.caltech.edu> writes:
>> Keith Thompson <kst-u@mib.org> writes:
>>> Tim Rentsch <txr@alumni.caltech.edu> writes:
>>> [...]
>>>>                   Related to that, the value ERANGE for errno
>>>> on line 51 is not really the best choice here, since the
>>>> problem is not a range error but a domain error.  Using EINVAL
>>>> is a better match to the actual circumstance, and also is more
>>>> in keeping (again AIUI) with POSIX and with other
>>>> implementations.
>>>
>>> But EINVAL is not one of the 3 errno values defined in the
>>> standard for <errno.h>, so you can't portably assume that it
>>> exists.  (The portable values are EDOM, EILSEQ, and ERANGE;
>>> EDOM is used only in <math.h>.)
>>
>> Right, I should have mentioned that.  I would hope that the
>> environment for his implementation includes EINVAL, but to
>> protect against the eventuality that it does not the error
>> code used can be based on a preprocessor test, eg,
>>
>>     #include <errno.h>
>>
>>     #ifdef EINVAL
>>     #  define  BAD_ARGUMENT  EINVAL
>>     #else
>>     #  define  BAD_ARGUMENT  EDOM
>>     #endif
>>
>>     /* Then use 'BAD_ARGUMENT' to set 'errno' */
>>
>> Alternatively the defined-ness of EINVAL can be tested and if it
>> is not present then a #error used to end the compilation.  There
>> is value in punishing implementations that don't have at least
>> minimal support for Posix.  :)
>
> I thought about that, but it means calling code potentially has to
> check for two different errno values.  The benefits of using a
> more sensible value on systems where it exists is outweighed,
> IMHO, by the benefits of consistency.
>
> If we want to make the C standard depend on the POSIX standard,
> then I suppose that could be done -- but I don't think it would
> fly.

I don't understand where you're coming from with these comments.
My suggestion is about one implemenation of the strto[u][l]l()
functions, not about the Standard.  As for consistency, it is IMO
far more important to be consistent with POSIX, where possible,
than it is to be consistent across all C implementations (ie, in
this particular situation).  Ideally enough systems will support
POSIX compliance so that EINVAL would be added as an required
macro in <errno.h> in the C standard;  that possibility does not
strike me at all as far-fetched, and would be a good result.


>> Incidentally, I see nothing wrong with re-using EDOM for this
>> purpose in the case when there is no EINVAL (and EDOM is a
>> better choice than ERANGE).
>
> Agreed.  Setting errno to EDOM in a case where the standard doesn't
> define the behavior is (a) conforming and (b) IMHO reasonable.

Another idea occurred to me.  Slightly modifying my earlier
suggestion, we could do something like this:

    #ifdef EINVAL
        return  errno = EINVAL,  0;
    #else
        fprintf( stderr, "Hey there..  you should get a better OS, "
                         "or run better programs "
                         "(or preferably both!).\n" );
        exit( EXIT_FAILURE );
    #endif

Now we have code that runs on all (hosted) implementations,
behaves reasonably on all systems that count, but still gives
predictable and useful results on those other systems.  :)


> The current discussion is about how to implement strtoll as
> specified by the current standard.

My impression is slightly different, in that the discussion is
only about one particular implementation of strtoll(), not
necessarily one suitable for all C implementations.  This may
explain the difference in our perspectives above.


> But I wouldn't mind seeing the standard updated to (a) define the
> behavior of stroll for an invalid base (I think that's the case
> we're talking about) and perhaps (b) to add EINVAL as one of the
> standard errno values.
>
> On the other hand, I strongly suspect that the value passed for
> base is almost always a constant, so defining the behavior for an
> incorrect value isn't a huge priority.

I agree on (b).  The more important undefined behavior is when
the first argument is null, which I expect both occurs more often
and is likely to have more severe consequences.

[toc] | [prev] | [next] | [standalone]

#59740

On 03.04.2015 14:28, jacobnavia wrote:
> Hi
> 
> I have written a document about the discusssion that we had in this
> group. It would be nice to give to newcomers here, so that they can read
> non-trivial code that is well documented.

Thanks, that will helpful when sdcc gets those functions.

Is there any particular reason you left out strtoimax() and strtoumax()?

Both approaches suggested in your pdf basically do the conversion using
the biggest integer. That is useful for code size when users use
multiple functions. But on the other strtol(), could be implemented with
just long multiplications; the approaches in the pdf would create a
dependency from strtol() on long long multiplications, which can be
quite expensive.

Philipp

[toc] | [prev] | [standalone]

Page 4 of 4 — ← Prev page 1 2 3 [4]

Back to top | Article view | comp.lang.c

csiph-web