bsnmp: fix undefined behaviour
ClosedPublic
Actions

Authored by ivy on Sun, Jul 27, 5:58 PM.

Details

Reviewers

des
glebius
emaste
harti

Commits

rG250bb6793a2b: bsnmp: fix undefined behaviour

Summary

Modifying and using a variable without an intervening sequence point
is undefined behaviour, which causes an error in the GCC build.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 65776
Build 62659: arc lint + arc unit

Event Timeline

ivy created this revision.Sun, Jul 27, 5:58 PM

Herald added a subscriber: imp. · View Herald TranscriptSun, Jul 27, 5:58 PM

ivy requested review of this revision.Sun, Jul 27, 5:58 PM

Harbormaster completed remote builds in B65776: Diff 159244.Sun, Jul 27, 5:58 PM

emaste added a subscriber: harti.Wed, Jul 30, 4:43 PM

Interesting, Clang can provide a similar warning, but it doesn't work w/ the array. For example,

fn.c:10:18: warning: unsequenced modification and access to 'p' [-Wunsequenced]
   10 |         return (port1 = ++p + strlen(p));
      |                         ^            ~

This revision is now accepted and ready to land.Wed, Jul 30, 4:57 PM

harti accepted this revision.Wed, Jul 30, 5:39 PM

des added inline comments.Thu, Jul 31, 1:15 PM

contrib/bsnmp/lib/snmpclient.c
2102	Alternatively, return (port[1] = strchr(++port[0], '\0'));

emaste added inline comments.Thu, Jul 31, 2:34 PM

contrib/bsnmp/lib/snmpclient.c
2102	That's valid, but at least for me I find it more confusing than @ivy's version

ivy added inline comments.Thu, Jul 31, 3:13 PM

contrib/bsnmp/lib/snmpclient.c
2102	this was why i split it into 3 lines: aside from being UB, it's also very difficult to read. des's version is slightly better than the original, but i see no reason to condense all this onto a single line.