19 Sep
2017
19 Sep
'17
8:34 p.m.
On Tue, Sep 19, 2017 at 08:31:52PM +0200, Steffen Nurpmeso wrote:
Random832 <random832(a)fastmail.com> wrote:
|On Tue, Sep 19, 2017, at 09:56, Larry McVoy wrote:
|> On Tue, Sep 19, 2017 at 03:53:59PM +0200, Steffen Nurpmeso wrote:
|>>|void
|>>|my_perror(char *file, int line, char *msg)
|>>|{
|>>| char *p = 0;
|>>| int save = errno;
|>>|
|>>| if (p = getenv("_BK_VERSION")) {
|>>| if (strneq(p, "bk-", 3)) p += 3;
|>>| fprintf(stderr, "%s:%d (%s): ", file, line, p);
|>>|} else {
|>>| fprintf(stderr, "%s:%d: ", file, line);
|>>|}
|>>| if (p = strerror(errno)) {
|>>| fprintf(stderr, "%s: %s\n", msg, p);
|>>|} else {
|>>| fprintf(stderr, "%s: errno=%d\n", msg, errno);
|>>|}
|>>| errno = save;
|>>|}
|>>|
|>>|libc should do that.
|>>
|>> That really made me wonder why "save" is not used, errno may
|>> eventually change along the way. Ok ok, but.. well.
|>
|> Huh? save is set with errno and then errno is restored to save. The
|> point of save is to do the library calls (which do syscalls which
|> could in theory change errno) and leave it the same as it was on
|> the way in.
|
|I think his point was that you should be passing save (rather than
|errno) to the strerror and the last printf, because the preceding
|library calls may have changed errno.
Well, if _you_ see it the WallStreetFighter style then it needs to
be said that an overwhelming, deadly, rather exterminating number
of points have been made. errno today is thread local storage, or
worse some pthread-specific macro expansion, i see multiple
call-ins to standard I/O which is potentially SMP-safe, stderr is
unbuffered, getenv() can end up doing a sequential walk on a flat
array with X number of strncmp() calls, in a context i assume the
variable itself is constant over the entire run of the software.
Also p=0 is C++ i think. There are two value assignments inside
a conditional statement without parenthesis. If the file
parameter comes from __FILE__ then that could and likely should be
constant string storage, msg looks likewise but possibly not as
bad. This code would benefit from an iteration.
--steffen
Feel free to send in a patch!