Date: Tue, 29 May 2018 12:16:00 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ed Maste <emaste@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r334291 - head/lib/libc/string Message-ID: <20180529113434.J1472@besplex.bde.org> In-Reply-To: <201805281829.w4SITFQB055010@repo.freebsd.org> References: <201805281829.w4SITFQB055010@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 28 May 2018, Ed Maste wrote:
> Log:
> strsep.3: don't silently ignore errors
>
> Reported by: bde
> MFC with: r334275
Thanks.
> Modified: head/lib/libc/string/strsep.3
> ==============================================================================
> --- head/lib/libc/string/strsep.3 Mon May 28 17:47:32 2018 (r334290)
> +++ head/lib/libc/string/strsep.3 Mon May 28 18:29:15 2018 (r334291)
> @@ -86,9 +86,10 @@ to parse a string, and prints each token in separate l
> char *token, *string, *tofree;
>
> tofree = string = strdup("abc,def,ghi");
> -if (string != NULL)
> - while ((token = strsep(&string, ",")) != NULL)
> - printf("%s\en", token);
> +if (string == NULL)
> + err(1, "strdup");
> +while ((token = strsep(&string, ",")) != NULL)
> + printf("%s\en", token);
>
> free(tofree);
> .Ed
This error handling is now good enough for small utilities. (I would
normally use xstrdup() and not repeat the comparison and err() in all
callers. Callers could print a more specific error message, but this
is too much for errors that "can't happen".)
But this example shouldn't be complicated by showing correct style and/or
technique for error handling. It should only try to show correct style and
technique for using strsep(). It only uses strdup() because it is a contrived
example with a fixed input string. String literals are const, so strsep()
can't be used directly on them, except in bad examples for compilers that
don't warn about this error. It uses strdup() to avoid this problem. Then
it has to worry about error checking, error handling, and freeing for strdup().
The example ends up with more code for strdup() than for strsep().
The example still has a technical error. It says that the string is parsed
and its tokens are printed. Actually, this only happens if strdup() didn't
fail. The description doesn't mention the error handling in any way.
Oops, there are many more technical errors. printf() can also fail. It
is more likely than strdup() to fail with ENOMEM. As usual, errors in it
are silenty ignored by not checking its return value. Most programs don't
check ferr() on any stdio stream before exit().
This example is followed by another one that is missing all of these
bugs, but has 3 related ones. Its string is named inputstring and
must be produced elsewhere, but it is declared in the example so it
is literally just uninitialized. The example doesn't preserve the
string, but writes NULs into it and points argv[] to the resulting
substrings. The description only gives a hint that the string will
be overwritten and and must live as long as the pointers. The problem
with not checking for errors from printf() is replaced by silently
ignoring the error of having more tokens than fit in argv[]. argv[]
has a fixed size of 10 elements to avoid complications.
Old versions of the man page have only the second example.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180529113434.J1472>