[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/4] localename: -Wtautological-pointer-compare

From: Paul Eggert
Subject: Re: [PATCH 1/4] localename: -Wtautological-pointer-compare
Date: Sat, 14 Jan 2023 19:02:23 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2

On 2023-01-14 03:00, Bruno Haible wrote:
Is that what you are worried about? Should I work on this?

No, please don't bother. I was partly confused by the situation and your email helped clear up some of it. Hope you don't mind this followup.

My confusion arose partly because I am accustomed to languages where the distinction between null and non-null pointers is checked statically and reliably, and I keep forgetting that with C, GCC and Clang are only poor approximations to that - though I hope the approximations are slowly getting better.

  * To avoid garbage-in - garbage-out behaviour, which in general encourages
    the presence of bugs.

Yes, that's a good thing, though in this particular case abort and dereferencing NULL have similar effects on typical platforms so this doesn't argue for one method or the other.

  * So that when an invalid argument gets passed and the developer happened
    to compile with -g, the debugger will point to the exact line of the
    abort(). When you say "the input should be checked anyway ... dynamically
    by the MMU", what the user would see is a SIGSEGV, and would start wondering
    whether the bug is in his code or in our code.

In my experience users don't care whether the application died due to SIGABRT or to SIGSEGV.

Also, in my experience the debugger doesn't always point to the exact line of the abort(). For example, if there are two abort() calls in the same function they are routinely coalesced. And come to think of it, I have somewhat better luck with gdb's reports of SIGSEGV from dereferencing null pointers (though there are obviously issues there too) than I do from abort() calls.

To give a different example: I wouldn't bother with the following code (where M and N are int arguments to a function):

    if (n == 0)
      abort ();
    if (n == -1 && m < -INT_MAX)
      abort ();
    return m / n;

and would instead write this:

    return m / n;

as the user and debugging experiences are similar and the shorter form simplifies code maintenance. Sure, the longer form is safer for oddball platforms, but it's not worth the aggravation.

  * For documentation purposes: So that it's clear that we have considered
    the doc of the function, and NULL at this particular place is invalid.

I'm willing to trust the intelligence of the reader to know that if a function computes *P, then it's invalid for P to be null.

Admittedly there's a lot of low-quality code out there where this isn't true, but here I'm focusing on the relatively high-quality code in Gnulib and GNU apps.

    (This is not evident: For example, does glibc's error_at_line() support a
    NULL fname argument? Hard to say from

The documentation is indeed unclear there, but this sort of thing is inevitable in any large system using C. For each such function where the spec is unclear, we should document whether a null pointer arg is allowed, and if it's not allowed the implementation of the function shouldn't need to worry about null pointer args (implementers already have too much to worry about, and part of the point of an API is simplification).

reply via email to

[Prev in Thread] Current Thread [Next in Thread]