bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: Bruno Haible <bruno@clisp.org>, bug-gnulib@gnu.org
Subject: Re: [PATCH 1/4] localename: -Wtautological-pointer-compare
Date: Sat, 14 Jan 2023 19:02:23 -0800	[thread overview]
Message-ID: <ca725b9f-b97d-815d-5c1d-a6851a0021b1@cs.ucla.edu> (raw)
In-Reply-To: <8041574.c9vzh5UkMf@nimes>

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 

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
>     <https://www.gnu.org/software/libc/manual/html_node/Error-Messages.html>.)

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 

  reply	other threads:[~2023-01-15  3:02 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-13 20:17 [PATCH 1/4] localename: -Wtautological-pointer-compare Paul Eggert
2023-01-13 20:17 ` [PATCH 2/4] Don’t use alloc_size with xlclang 16.1 Paul Eggert
2023-01-13 20:17 ` [PATCH 3/4] assert-h: fix configure comment-out Paul Eggert
2023-01-13 20:17 ` [PATCH 4/4] assert-h: suppress xlclang 16.1 false alarms Paul Eggert
2023-01-13 22:59 ` [PATCH 1/4] localename: -Wtautological-pointer-compare Bruno Haible
2023-01-13 23:36   ` Paul Eggert
2023-01-14 11:00     ` Bruno Haible
2023-01-15  3:02       ` Paul Eggert [this message]
2023-01-15 22:03         ` Bruno Haible
2023-01-16  0:15           ` Paul Eggert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information: https://lists.gnu.org/mailman/listinfo/bug-gnulib

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ca725b9f-b97d-815d-5c1d-a6851a0021b1@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=bruno@clisp.org \
    --cc=bug-gnulib@gnu.org \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).