From owner-svn-src-all@freebsd.org Tue May 29 02:16:10 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 37596F78927; Tue, 29 May 2018 02:16:10 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id AB80370500; Tue, 29 May 2018 02:16:08 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id 4BACFD69643; Tue, 29 May 2018 12:16:00 +1000 (AEST) Date: Tue, 29 May 2018 12:16:00 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Ed Maste cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r334291 - head/lib/libc/string In-Reply-To: <201805281829.w4SITFQB055010@repo.freebsd.org> Message-ID: <20180529113434.J1472@besplex.bde.org> References: <201805281829.w4SITFQB055010@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=cIaQihWN c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=tu7e0SMpR-IVpYf90UQA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 May 2018 02:16:10 -0000 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