On Mon, Feb 06, 2017 at 02:04:52PM -0800, Evan Gates wrote: > Hiltjo Posthuma <[email protected]> wrote: > > > Probably a false positive, but for ed.c it looks ok to me. > > > > The join.c patch does not look ok to me (clang doesn't detect that > > eprintf() exists the program), so it's a false positive. > > This is where the _Noreturn or noreturn attribute is handy. > Unfortunately that's C11. > > It seems ed.c is the same problem, error() has a longjmp() instead of > return causing a false positive. > > I do like getting rid of warnings but I'm not particularly fond of > changes purely for that reason. > > As for join.c, if we do care about getting rid of warnings we can > rewrite to take advantage of the initialized values instead of assigning > again when s is "0". Although I'm not sure the intent is as clear this > way. Example patch below. > > I'd like to hear more thoughts on this, if the community's response is > overwhelmingly one way or the other I'll gladly accept the changes. > > ----- 8< ----- 8< ----- > > From 5eae9c0bd10c6da57fcd8527e7bbc6c23cda4b08 Mon Sep 17 00:00:00 2001 > From: Evan Gates <[email protected]> > Date: Mon, 6 Feb 2017 13:50:23 -0800 > Subject: [PATCH] Initialize to avoid false positive clang warning > > clang doesn't recognize eprintf exits and warns about possibily > uninitialized values. Initialize the values and rearrange the > conditionals to avoid redundant assignment. > --- > join.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/join.c b/join.c > index d3e2343..e62bb8f 100644 > --- a/join.c > +++ b/join.c > @@ -314,16 +314,13 @@ static struct spec * > makespec(char *s) > { > struct spec *sp; > - int fileno; > - size_t fldno; > + int fileno = 0; /* default to "0", join field must be 0 and nothing > else */ > + size_t fldno = 0; > > - if (!strcmp(s, "0")) { /* join field must be 0 and nothing else */ > - fileno = 0; > - fldno = 0; > - } else if ((s[0] == '1' || s[0] == '2') && s[1] == '.') { > + if ((s[0] == '1' || s[0] == '2') && s[1] == '.') { > fileno = s[0] - '0'; > fldno = estrtonum(&s[2], 1, MIN(LLONG_MAX, SIZE_MAX)) - 1; > - } else { > + } else if (strcmp(s, "0")){ > eprintf("%s: invalid format\n", s); > } > > -- > 2.11.0 > >
Looks ok to me. -- Kind regards, Hiltjo
