Fix two clang 3.6.0 warnings in usr.sbin/syslogd
ClosedPublic
Actions

Authored by dim on Jan 29 2015, 7:34 AM.

Details

Reviewers

ae
jilles
• ian

Commits

rS278437: MFC r278297:
rS278297: Fix two clang 3.6.0 warnings in usr.sbin/syslogd:

Summary

Fix two clang 3.6.0 warnings in usr.sbin/syslogd:

usr.sbin/syslogd/syslogd.c:1023:10: error: address of array 'f->f_prevline' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
                    f->f_prevline && !strcmp(msg, f->f_prevline) &&
                    ~~~^~~~~~~~~~
usr.sbin/syslogd/syslogd.c:1178:16: error: address of array 'f->f_prevline' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
        } else if (f->f_prevline) {
               ~~  ~~~^~~~~~~~~~

In both cases, the f_prevline field of struct filed is a char array, so it can never be null.

However, I think the intent was to check whether the string was empty, so check f->f_prevline[0] instead. (See also line 1056, where f->f_prevline[0] is explicitly set to zero.)

Test Plan

Compile and run.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

dim updated this revision to Diff 3507.Jan 29 2015, 7:34 AM

dim retitled this revision from to Fix two clang 3.6.0 warnings in usr.sbin/syslogd.

dim updated this object.

dim edited the test plan for this revision. (Show Details)

dim added reviewers: jilles, • ian, ae.

I disagree with your analysis. f_prevline has always been an array dating back to r1553. In r174533, obrien mistakenly assumed it was a pointer to an array and added null-pointer checks, his commit message even says so, see

https://svnweb.freebsd.org/base?view=revision&revision=174533

So the right fix is just to remove the mistaken NULL checks. That commit message also says something about trying to free f_prevline before exit, I guess we better make sure it's not doing that too.

In D1716#3, @ian wrote:

I disagree with your analysis. f_prevline has always been an array dating back to r1553. In r174533, obrien mistakenly assumed it was a pointer to an array and added null-pointer checks, his commit message even says so, see

https://svnweb.freebsd.org/base?view=revision&revision=174533

So the right fix is just to remove the mistaken NULL checks. That commit message also says something about trying to free f_prevline before exit, I guess we better make sure it's not doing that too.

Yes, I guess he just copy/pasted a commit message from Juniper there, and maybe Juniper had their own version with a dynamically allocated f_prevline.

I don't see it trying to free f_prevline anywhere, just a free(wmsg) at the end of fprintlog().

Remove unnecessary f_prevline checks.

So, is this now good to commit?

Looks good to me.

This revision is now accepted and ready to land.Feb 5 2015, 10:12 PM

Closed by commit rS278297 (authored by @dim).

Revision Contents
Changeset List

Path

Size

usr.sbin/

syslogd/

syslogd.c

4 lines

Diff 3507

View Options

usr.sbin/syslogd/syslogd.c

Fix two clang 3.6.0 warnings in usr.sbin/syslogdClosedPublicActions