«

Simon Biber <> wrote:
> "Zoran Cutura" <> wrote:
>> Simon Biber <> wrote:
>
>> But probably I've missed something.
>
> If the value returned by strlen is greater than INT_MAX,
> the comparison will always be true. Then after some time
> while incrementing through the loop, i will equal INT_MAX,
> it will be incremented once more, and overflow will occur.
> This is arithmetic overflow in a signed type, not a
> conversion, and so it is undefined behaviour.

Ahh, I see it's not in the comparison but in the increment.

>

--
Z ()
"LISP is worth learning for the profound enlightenment experience
you will have when you finally get it; that experience will make you
a better programmer for the rest of your days." -- Eric S. Raymond

In <3ef92f55$0$8262$> "Simon Biber" <> writes:

>"Dan Pop" <> wrote:
>> "Simon Biber" <> wrote:
>> > The warnings triggered by -W are often more stylistic things,
>> > or reminders that some practise may be a bit dangerous in
>> > some situations (even if in the specific case it is actually
>> > correct). I quite like those warnings, and prefer to modify
>> > my code to work around them rather than disable them,
>> > wherever possible.
>>
>> I find them a nuisance and see no good reason for working
>> around them. The following is a trivial example where the
>> compiler should trust the programmer:
>>
>> fangorn:~/tmp 336> cat test.c
>> #include <string.h>
>>
>> void foo(int arg)
>> {
>> int i;
>> for (i = 0; i < strlen("abcde"); i++) /* do something */ ;
>> }
>> fangorn:~/tmp 337> gcc -c -Wall test.c
>> fangorn:~/tmp 338> gcc -c -Wall -W test.c
>> test.c: In function `foo':
>> test.c:6: warning: comparison between signed and unsigned
>
>There is very good reason for the warning here as it has
>uncovered a potential bug in your code. If the value
>returned by strlen is greater than INT_MAX, your program
>has undefined behaviour due to overflow.

Since when is allowed INT_MAX to be less than 5? I may have more
information than the compiler, therefore the compiler must trust me.

>You should always use size_t for loop iterators whose bound
>is expressed in size_t. This includes arrays when you use
> sizeof foo / sizeof *foo
>as the bound.

Sheer bullshit! If I *know* that int is enough, there's NO reason for
using size_t.

>> test.c:3: warning: unused parameter `arg'
>>
>> I can see NO good reason for doing something with arg to keep
>> -W silent and I cannot drop it from the function interface
>> (which is externally imposed).
>
>In my experience externally imposed interfaces are uncommon,

Have a look at the specification of signal() sometime.

>and I see the odd cast to void as useful self-documentation.
>It lets the reader of the code immediately see that you
>intentially do not use the value of the argument.

Or he might start wondering why I am doing such a silly thing as casting
an otherwise unused argument to void.

If I don't use the value of the argument (as is usually the case with
my signal handlers), what is the obvious conclusion?

>> Likewise, I don't want to declare i as unsigned for the
>> *sole* reason of shutting up -W (or cast the return value
>> of strlen). I only use unsigned variables when I have a
>> good reason for that, because of reasons discussed in a
>> different thread.
>
>But there is a very good reason to use a size_t variable here!

Nope! Single letter variables are often reused as loop counters and
temporary variables. More often than not, the type int is more
appropriate than an unsigned type and the decision should be mine, not
compiler's, anyway.

>So, I would write:
> #include <string.h>
>
> void foo(int arg)
> {
> (void)arg;
> for (size_t i = 0, n = strlen("abcde"); i < n; i++)
> {
> /* do something */
> }
> }
>for which `gcc -c -std=c99 -pedantic -Wall -W -O2 warn.c`
>gives no warnings.

That's precisely my point: why bother with options that *force* you to
code around them? What guarantee do you have that *another* compiler
won't complain about the useless statement (void)arg; ?

Apart from that, your code is not portable to *most* C compilers, for no
*good* reason at all.

Dan
--
Dan Pop
DESY Zeuthen, RZ group
Email:

Richard Bos wrote:
> Richard Heathfield <> wrote:
>>Neil Cerutti wrote:
....
>>>Here's a slightly less trivial example, that allows me to make my
>>>point:
>>>
>>>void foo(int arg, char *s)
>>>{
>>> int i; /* wrong */
>>> for (i = 0; i < strlen(s); ++i) do_something(s[i]);
>>>}
>>>
>>>Using this example, it becomes possible to say that i ought to be
>>>of type size_t, making it's range equal to strlen's range.
>>
>>And so it should. But don't you think it's better to move the strlen out of
>>the loop?
>
> No. If do_something() can set s[i] to '\0', [...]

It's not possible, if do_something is a function. And if do_something
is a macro, it does not have proper notation.

Jirka

"Dan Pop" <> wrote:
> "Simon Biber" <> writes:
> > There is very good reason for the warning here as it
> > has uncovered a potential bug in your code. If the
> > value returned by strlen is greater than INT_MAX,
> > your program has undefined behaviour due to overflow.
>
> Since when is allowed INT_MAX to be less than 5? I may
> have more information than the compiler, therefore the
> compiler must trust me.

I said "potential bug". There is no bug in your code,
but in most cases when looping through a string there
is the possibility of it being of any length, and it
is quite possible it could exceed INT_MAX.

You clearly like to write code that gets the job done but
leaves no room for expansion or re-use. I prefer to code
in the most robust way available.

> >You should always use size_t for loop iterators whose
> >bound is expressed in size_t. This includes arrays
> >when you use
> > sizeof foo / sizeof *foo
> >as the bound.
>
> Sheer bullshit! If I *know* that int is enough, there's
> NO reason for using size_t.

Again, you know that int is enough now, but who knows what
changes you or other people may wish to make to your code
in the future? It is better to do things properly now so
that future expansion will be painless.

> >In my experience externally imposed interfaces are uncommon,
>
> Have a look at the specification of signal() sometime.

Name some common, portable, standard-conforming uses of signal().

> >and I see the odd cast to void as useful self-documentation.
> >It lets the reader of the code immediately see that you
> >intentially do not use the value of the argument.
>
> Or he might start wondering why I am doing such a silly thing
> as casting an otherwise unused argument to void.

The cast to void is a fairly well-known signal for "I don't
care what value this expression has".

> If I don't use the value of the argument (as is usually the
> case with my signal handlers), what is the obvious conclusion?

If the function is long and/or complicated it may not be obvious
at a glance whether or not each argument is actually used.
Furthermore the non-use of an argument is so often a mistake that
a reader is forced to reevaluate whether or not you really did
mean to not use it the argument.

> > But there is a very good reason to use a size_t
> > variable here!
>
> Nope! Single letter variables are often reused as loop
> counters and temporary variables. More often than not,
> the type int is more appropriate than an unsigned type
> and the decision should be mine, not compiler's, anyway.

Variables should be reduced to the minimum scope necessary.
Single letter variables should not be reused, this creates
confusion over whether their value is significant between
uses.

I like the for(type i=0; i<n; i++) form from C99, where the
variable goes out of scope immediately on exit from the loop.

> >So, I would write:
> > #include <string.h>
> >
> > void foo(int arg)
> > {
> > (void)arg;
> > for (size_t i = 0, n = strlen("abcde"); i < n; i++)
> > {
> > /* do something */
> > }
> > }
> >for which `gcc -c -std=c99 -pedantic -Wall -W -O2 warn.c`
> >gives no warnings.
>
> That's precisely my point: why bother with options that
> *force* you to code around them?

I don't feel forced to code around it! I believe it is good
style to code in such a way that the warnings don't come up.

> What guarantee do you have that *another* compiler
> won't complain about the useless statement (void)arg; ?

None; a compiler is allowed to complain about anything.
However, in my experience compilers universally take a
cast as a sign to shut up.

> Apart from that, your code is not portable to *most* C
> compilers, for no *good* reason at all.

Bogus. My code is portable to *all* C compilers.

Most of the compilers that used to be C compilers,
are now obsolete. C99 replaced C89. C89 is not C.

Come to think of it, since I started programming in C
after C99 was released, I've never used a C compiler!



--
Simon.

Simon Biber wrote:
[...]
> None; a compiler is allowed to complain about anything.
> However, in my experience compilers universally take a
> cast as a sign to shut up.

Intel's C compiler does not, so "universally" is too strong.

int i = 42;
char *p = (char *)i;

main.c(155): remark #171: invalid type conversion: "int" to "char *"
char *p = (char *)i;
^

Jirka

On Wed, 25 Jun 2003 08:01:43 GMT,
Richard Bos <> wrote
in Msg. <>

> No. If do_something() can set s[i] to '\0', the loop will abort next
> time, and this effect may have been intentional. If do_something() never
> changes s[i], or rather, never sets it to the null character, the
> compiler can figure that out and optimise the strlen() call away itself.

Not knowing much about modern optimizing compilers, and also not wanting
to drag this thread off-topic, but just out of curiosity: How far do
compilers go when looking at such code? Do the optimizations cross
function calls?

> And leaving the strlen() where it is makes for a semantically better
> (read: more naturally expressed) program.

True. But in my mind, strlen() is still nothing but a dumb loop that
counts all non-zero characters from the beginning of a string each time it
is called, so buffering strlen()'s result in some local variable whenever
I want to use a (constant-length) string's length more than once is a
habit of mine that looks pretty unbreakable.

--Daniel

--
"With me is nothing wrong! And with you?" (from r.a.m.p)

On 25 Jun 2003 11:19:54 GMT,
Dan Pop <> wrote
in Msg. <bdc0gq$eoc$>

> Or he might start wondering why I am doing such a silly thing as casting
> an otherwise unused argument to void.
>
> If I don't use the value of the argument (as is usually the case with
> my signal handlers), what is the obvious conclusion?

That you don't need it. But if you like to compile with all warnings set
to full blast (like I do), it is all too easy to miss the one important
diagnostic in a screenful of "unused argument" warnings. I once wasted
half an afternoon looking for a bug which was indeed caused by an unused
argument, and I only found it by (void)ing the intentionally unused
arguments in two dozen signal handlers.

>>for which `gcc -c -std=c99 -pedantic -Wall -W -O2 warn.c`
>>gives no warnings.
>
> That's precisely my point: why bother with options that *force* you to
> code around them?

Reason given above.

> What guarantee do you have that *another* compiler
> won't complain about the useless statement (void)arg; ?

None. If I wanted to code in such a way that all compilers happily ate my
code without spitting out warnings, I'd probably have to define a macro
THROWAWAY(arg) which expands to different things for different compilers.

--Daniel

--
"With me is nothing wrong! And with you?" (from r.a.m.p)

Jirka Klaue <> wrote:

> Richard Bos wrote:
> > Richard Heathfield <> wrote:
> >>And so it should. But don't you think it's better to move the strlen out of
> >>the loop?
> >
> > No. If do_something() can set s[i] to '\0', [...]
>
> It's not possible, if do_something is a function. And if do_something
> is a macro, it does not have proper notation.

True. But people _do_ use all-lower-caps macros. All the more reason to
leave the strlen() in and trust the optimiser, just to spite that kind
of person <g>.

Ricahrd

Daniel Haude <> wrote:

> Richard Bos <> wrote
>
> > And leaving the strlen() where it is makes for a semantically better
> > (read: more naturally expressed) program.
>
> True. But in my mind, strlen() is still nothing but a dumb loop that
> counts all non-zero characters from the beginning of a string each time it
> is called,

Ah. And this is where you're wrong. strlen() is a function that return
the length of the string. That it is entirely likely that it does this
by counting characters should not prevent you from thinking on a higher
level. After all, _you're_ not a microprocessor.

Richard

In <3ef99341$0$8262$> "Simon Biber" <> writes:

>"Dan Pop" <> wrote:
>> "Simon Biber" <> writes:
>> > There is very good reason for the warning here as it
>> > has uncovered a potential bug in your code. If the
>> > value returned by strlen is greater than INT_MAX,
>> > your program has undefined behaviour due to overflow.
>>
>> Since when is allowed INT_MAX to be less than 5? I may
>> have more information than the compiler, therefore the
>> compiler must trust me.
>
>I said "potential bug". There is no bug in your code,
>but in most cases when looping through a string there
>is the possibility of it being of any length, and it
>is quite possible it could exceed INT_MAX.

But the point is that I have additional information that rules out this
*theoretical* possibility. Therefore, there is no good reason for using
size_t.

Apart from that, when was the last time you used a string whose size
exceeded INT_MAX, so that the "quite possible" in your assertion is
justified?

>You clearly like to write code that gets the job done but
>leaves no room for expansion or re-use. I prefer to code
>in the most robust way available.

Bullshit. I've never had any problem reusing or expanding my code.
If I decide, at design time, that the right type for a variable is int,
then I know what I'm doing. The compiler has no business to question my
decision.

>> >You should always use size_t for loop iterators whose
>> >bound is expressed in size_t. This includes arrays
>> >when you use
>> > sizeof foo / sizeof *foo
>> >as the bound.
>>
>> Sheer bullshit! If I *know* that int is enough, there's
>> NO reason for using size_t.
>
>Again, you know that int is enough now, but who knows what
>changes you or other people may wish to make to your code
>in the future? It is better to do things properly now so
>that future expansion will be painless.

You're missing the point! Unsigned variables have plenty of pitfalls
and one of them could affect the future changes (especially if they are
done by someone with less experience). By choosing the type int, where
it is the *right* type, I'm ensuring that even a non-experienced
programmer can maintain the code. If you still can't see my point,
replace strlen(something) by sizeof(double) in my original example.
In theory, sizeof(double) could exceed 32767, yet I'm perfectly willing
to ignore this possibility.

>> >In my experience externally imposed interfaces are uncommon,
>>
>> Have a look at the specification of signal() sometime.
>
>Name some common, portable, standard-conforming uses of signal().

#include <signal.h>

volatile sig_atomic_t interrupt;

void handler(int signo)
{
interrupt = 1;
}
....
signal(SIGINT, handler);
signal(SIGTERM, handler);

>> >and I see the odd cast to void as useful self-documentation.
>> >It lets the reader of the code immediately see that you
>> >intentially do not use the value of the argument.
>>
>> Or he might start wondering why I am doing such a silly thing
>> as casting an otherwise unused argument to void.
>
>The cast to void is a fairly well-known signal for "I don't
>care what value this expression has".

Chapter and verse, please. Or other source for this widespread knowledge.

>> If I don't use the value of the argument (as is usually the
>> case with my signal handlers), what is the obvious conclusion?
>
>If the function is long and/or complicated it may not be obvious
>at a glance whether or not each argument is actually used.

Why should anyone care?

>Furthermore the non-use of an argument is so often a mistake that
^^^^^^^^
>a reader is forced to reevaluate whether or not you really did
>mean to not use it the argument.

Care to produce some supporting data for "so often"? I've never ignored
an argument by mistake, each and every time it was a deliberate decision.

>> > But there is a very good reason to use a size_t
>> > variable here!
>>
>> Nope! Single letter variables are often reused as loop
>> counters and temporary variables. More often than not,
>> the type int is more appropriate than an unsigned type
>> and the decision should be mine, not compiler's, anyway.
>
>Variables should be reduced to the minimum scope necessary.

That would mean opening plenty of blocks for no good reason. IIRC, it
was E.R. Tisdale advocating this approach.

>Single letter variables should not be reused, this creates
>confusion over whether their value is significant between
>uses.

No such confusion is possible, when the reuse starts be assigning a new
value. It would be downright idiotic to use a different loop control
variable for each independent loop in a function.

>I like the for(type i=0; i<n; i++) form from C99, where the
>variable goes out of scope immediately on exit from the loop.

It's non-portable. Some people do care about portability, even if you
don't.

>> >So, I would write:
>> > #include <string.h>
>> >
>> > void foo(int arg)
>> > {
>> > (void)arg;
>> > for (size_t i = 0, n = strlen("abcde"); i < n; i++)
>> > {
>> > /* do something */
>> > }
>> > }
>> >for which `gcc -c -std=c99 -pedantic -Wall -W -O2 warn.c`
>> >gives no warnings.
>>
>> That's precisely my point: why bother with options that
>> *force* you to code around them?
>
>I don't feel forced to code around it! I believe it is good
>style to code in such a way that the warnings don't come up.

I don't. While the unused argument issue is harmless, the gratuitous
usage of unsigned variables is NOT.

>> What guarantee do you have that *another* compiler
>> won't complain about the useless statement (void)arg; ?
>
>None; a compiler is allowed to complain about anything.
>However, in my experience compilers universally take a
>cast as a sign to shut up.

Is it "your experience" or "universally"?

>> Apart from that, your code is not portable to *most* C
>> compilers, for no *good* reason at all.
>
>Bogus. My code is portable to *all* C compilers.

By a bogus definition of C. Your code is non-portable to most of
the real world C compilers, period.

>Most of the compilers that used to be C compilers,
>are now obsolete. C99 replaced C89. C89 is not C.

You're severely confused. One ISO standard replaced another. Up to now,
this had no significant impact either on the industry or on the academia.
The C programming community at large is ignoring the C99 specification.

Furthermore, as attested by comp.std.c, copies of the ISO C90 standard
are still in constant demand (and at least one national standardisation
organisation, BSI, is currently selling them).

The value of an ISO standard is determined by its impact on the industry
that's supposed to use it. Until now, C99 had practically none.

Dan
--
Dan Pop
DESY Zeuthen, RZ group
Email: