search

intel / openlldp

Other
54 stars 42 forks source link

Coverity fixes #21

Closed hreinecke closed 4 years ago

praeluceo commented 5 years ago

With all patches applied there are few compilation errors all related to Wformat-overflow when using gcc version 7.3.1 20180323:

lldpad_shm.c: In function ‘lldpad_shm_set_msap’: lldpad_shm.c:210:41: error: ‘__builtin___sprintf_chk’ may write a terminating nul past the end of the destination [-Werror=format-overflow=] sprintf(shmaddr->ent[i].ifname, "%.*s", ^ In file included from /usr/include/stdio.h:862:0, from lldpad_shm.c:28:

I think this is the troublesome patch:

https://github.com/intel/openlldp/pull/21/commits/c9d2f87fa948f30103487a09578894763be0cbde?utf8=%E2%9C%93&diff=unified

commit c9d2f87fa948f30103487a09578894763be0cbde Author: Hannes Reinecke hare@suse.de Date: Tue Jun 21 13:04:07 2016 +0200 Use correct interface name size

diff --git a/include/clif_msgs.h b/include/clif_msgs.h index a5b10f6..8ea3ff0 100644 --- a/include/clif_msgs.h +++ b/include/clif_msgs.h @@ -107,7 +107,7 @@ struct cmd { u32 ops; u32 tlvid; __u8 type;

From that I can gather https://gcc.gnu.org/gcc-7/changes.html increasing the dest buffer size by one (as it was before) is a valid solution to -Wformat-overflow=

So please review your commit, this is a large patchset that doesn't impact functionality.

praeluceo commented 5 years ago

This patch fails to compile on SLES 15 (gcc 7.3.1) and on Fedora 28+ (gcc 8).

praeluceo commented 5 years ago

@hreinecke do you have any update on this patch? It should be broken up into smaller PRs, and it will need the merge conflict to be resolved as well.

hreinecke commented 4 years ago

You are correct, I'd rather retract this series and post a new pull request.