| Message ID | 20170713172145.GU29918@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
On 07/13/2017 07:21 PM, Ben Pfaff wrote: > On Thu, Jul 13, 2017 at 04:29:33PM +0200, Timothy Redaelli wrote: >> test_snprintf function (tests/test-util.c) tests snprintf with shorter length, >> but this emit a warning on GCC 7.0 or later. >> >> This commit disables that warning on tests only. >> >> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> > > How about disabling it just for those lines of code? Good idea, it's surely better to limit the portion of disabled warning in order to maximize the effectiveness of the warning itself. LGTM
On Fri, Jul 14, 2017 at 4:46 AM, Timothy M. Redaelli <tredaelli@redhat.com> wrote: > On 07/13/2017 07:21 PM, Ben Pfaff wrote: >> On Thu, Jul 13, 2017 at 04:29:33PM +0200, Timothy Redaelli wrote: >>> test_snprintf function (tests/test-util.c) tests snprintf with shorter length, >>> but this emit a warning on GCC 7.0 or later. >>> >>> This commit disables that warning on tests only. >>> >>> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> >> >> How about disabling it just for those lines of code? > > Good idea, it's surely better to limit the portion of disabled warning > in order to maximize the effectiveness of the warning itself. > > LGTM Oops, I applied the original version of this patch from patchwork before seeing Ben's alternative. Patchwork turned Ben's reply into a patch on patchwork instead of associating it with the original, so I missed it. I'll squash a revert of the original into Ben's patch here and apply that, too.
On Fri, Jul 14, 2017 at 09:26:36PM -0400, Russell Bryant wrote: > On Fri, Jul 14, 2017 at 4:46 AM, Timothy M. Redaelli > <tredaelli@redhat.com> wrote: > > On 07/13/2017 07:21 PM, Ben Pfaff wrote: > >> On Thu, Jul 13, 2017 at 04:29:33PM +0200, Timothy Redaelli wrote: > >>> test_snprintf function (tests/test-util.c) tests snprintf with shorter length, > >>> but this emit a warning on GCC 7.0 or later. > >>> > >>> This commit disables that warning on tests only. > >>> > >>> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> > >> > >> How about disabling it just for those lines of code? > > > > Good idea, it's surely better to limit the portion of disabled warning > > in order to maximize the effectiveness of the warning itself. > > > > LGTM > > Oops, I applied the original version of this patch from patchwork > before seeing Ben's alternative. > > Patchwork turned Ben's reply into a patch on patchwork instead of > associating it with the original, so I missed it. Yeah, that's a feature of patchwork with mixed results. > I'll squash a revert of the original into Ben's patch here and apply that, too. Sounds good.
diff --git a/tests/test-util.c b/tests/test-util.c index e37c722829e0..9222355ec1b0 100644 --- a/tests/test-util.c +++ b/tests/test-util.c @@ -1116,11 +1116,21 @@ test_snprintf(struct ovs_cmdl_context *ctx OVS_UNUSED) { char s[16]; +#if __GNUC__ >= 7 + /* GCC 7+ warns about the following calls that truncate a string using + * snprintf(). We're testing that truncation works properly, so + * temporarily disable the warning. */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-truncation" +#endif ovs_assert(snprintf(s, 4, "abcde") == 5); ovs_assert(!strcmp(s, "abc")); ovs_assert(snprintf(s, 5, "abcde") == 5); ovs_assert(!strcmp(s, "abcd")); +#if __GNUC__ >= 7 +#pragma GCC diagnostic pop +#endif ovs_assert(snprintf(s, 6, "abcde") == 5); ovs_assert(!strcmp(s, "abcde"));