groff
[Top][All Lists]
Advanced

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

Re: Use `strsave()`, not `strdup()`.


From: Ralph Corderoy
Subject: Re: Use `strsave()`, not `strdup()`.
Date: Mon, 08 Nov 2021 14:26:40 +0000

Hi Ingo,

> If the intention of strsave() was to replace strdup(), then the
> correct implementation would have been
>
>       size_t sz = strlen(s) + 1;
>       char *p = malloc(sz);
>       if (p != NULL)
>               memcpy(p, s, sz);
>       return p:

An estrdup() would be:

    if (!s)
        abort();
    char *n = strdup(s);
    if (!n)
        abort();
    return n;

This is simpler to verify IMO.

- It explicitly checks for the source string being a null pointer
  because dereferencing null doesn't guarantee a crash on all platforms;
  on my first C compiler platform, it gave the contents of memory
  location 0.
- The abort()s would really write(2) to stderr first; Brandon's
  suggestion of fputs(3) wrongly assumes it won't allocate memory.
- It does rely on a strdup() being present, either naturally or through
  a local stand-in, but the advantage of doing so is to break
  comprehension down into two separate parts.
- By calling one standard-library function instead of three in the
  common path, it's easier for the compiler to provide better
  performance.
- And it avoids the size_t overflow when adding one.  ;-)

> I don't think real-world implementations [of printf] allocate memory
> if you refrain from relying on wide character to multibyte character
> conversions.  Why should they?

I've experienced several which allocate.  One example is to get a stdio
buffer to print to as it's the first print such.

>       if ((p = malloc(sz)) == NULL) {
>               perror(__progname);
>               exit(1);
>       }

I don't know what value sz has in that case.  Ideally, it would be
write(2)-nd.  At least abort(3) might capture it.

> The message
>
>   __progname: Cannot allocate memory
>
> contains all information the user needs.

This is incorrect.  What bit of progname?  What was it trying to do?
How much was it trying to allocate?  Is that a reasonable amount or some
silly high amount which suggests memory has been corrupted rather than a
faulty calculation.

> I think printing the names of source code files, the names of
> functions in the source code, or line numbers of call sites in source
> code files is bad practice.  Such information is totally irrelevant
> for normal users, it is confusing and distracting, and it just makes
> the program look as if it were written for nerds rather than for
> users.  It does not even help developers.

If a user can report
    glyph.c:42:estrdup failed: 31415 'foo bar xyzzy...'
to the list for a sporadic bug which they can't reliably reproduce then
it far more helpful than
    __progname: Cannot allocate memory

> Note that
>   fatal("strdup: %1", strerror(errno));
> would be pointless.  It is completely irrelevant for the user whether
> it was malloc(3) or strdup(3) or whatever other allocating function
> that failed.  All that matters is "Cannot allocate memory".

The user reports up.  It may be a one-shot chance.

-- 
Cheers, Ralph.



reply via email to

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