Skip to content

Commit

Permalink
configure: Stop avoiding -Wformat-zero-length.
Browse files Browse the repository at this point in the history
Debian likes to enable -Wformat-zero-length, even over our code trying to
disable it.  It isn't too hard to make our code warning-free against this
option, so this commit both stops disabling it and fixes the warnings.

The first fix is to change set_subprogram_name() to take a plain string
instead of a format string, and to adjust its few callers.  This fixes one
warning since one of those callers passed in an empty string.

The second fix is to remove a test for ovs_scan() against an empty string.
I couldn't find a way to avoid a warning for this test, and it isn't too
valuable in any case.

This allows us to drop filtering for -Wformat from the Debian rules file,
so this commit removes it.

Signed-off-by: Ben Pfaff <[email protected]>
  • Loading branch information
blp committed Jun 10, 2015
1 parent bdd7ecf commit 40e7cf5
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 31 deletions.
3 changes: 1 addition & 2 deletions configure.ac
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -144,7 +144,6 @@ OVS_ENABLE_OPTION([-Wextra])
OVS_ENABLE_OPTION([-Wno-sign-compare])
OVS_ENABLE_OPTION([-Wpointer-arith])
OVS_ENABLE_OPTION([-Wformat-security])
OVS_ENABLE_OPTION([-Wno-format-zero-length])
OVS_ENABLE_OPTION([-Wswitch-enum])
OVS_ENABLE_OPTION([-Wunused-parameter])
OVS_ENABLE_OPTION([-Wbad-function-cast])
Expand Down
7 changes: 0 additions & 7 deletions debian/rules
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,6 @@ endif
buildflags := $(shell if dpkg-buildflags --export=configure >/dev/null 2>&1; \
then dpkg-buildflags --export=configure; fi)

# dpkg-buildflags tends to turn on -Wformat, which is admirable, but
# the -Wformat-zero-length subset of that option triggers a couple of
# false positives in Open vSwitch so turn it right back off again.
# (We do this in configure.ac also, but the Debian buildflags override
# those.)
buildflags := $(patsubst -Wformat,-Wformat -Wno-format-zero-length,$(buildflags))

configure: configure-stamp
configure-stamp:
dh_testdir
Expand Down
6 changes: 4 additions & 2 deletions lib/ovs-thread.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2014 Nicira, Inc.
* Copyright (c) 2013, 2014, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -332,7 +332,9 @@ ovsthread_wrapper(void *aux_)

/* The order of the following calls is important, because
* ovsrcu_quiesce_end() saves a copy of the thread name. */
set_subprogram_name("%s%u", aux.name, id);
char *subprogram_name = xasprintf("%s%u", aux.name, id);
set_subprogram_name(subprogram_name);
free(subprogram_name);
ovsrcu_quiesce_end();

return aux.start(aux.arg);
Expand Down
21 changes: 5 additions & 16 deletions lib/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -500,24 +500,13 @@ get_subprogram_name(void)
return name ? name : "";
}

/* Sets the formatted value of 'format' as the name of the currently running
* thread or process. (This appears in log messages and may also be visible in
* system process listings and debuggers.) */
/* Sets 'subprogram_name' as the name of the currently running thread or
* process. (This appears in log messages and may also be visible in system
* process listings and debuggers.) */
void
set_subprogram_name(const char *format, ...)
set_subprogram_name(const char *subprogram_name)
{
char *pname;

if (format) {
va_list args;

va_start(args, format);
pname = xvasprintf(format, args);
va_end(args);
} else {
pname = xstrdup(program_name);
}

char *pname = xstrdup(subprogram_name ? subprogram_name : program_name);
free(subprogram_name_set(pname));

#if HAVE_GLIBC_PTHREAD_SETNAME_NP
Expand Down
4 changes: 2 additions & 2 deletions lib/util.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -263,7 +263,7 @@ extern "C" {
ovs_set_program_name(name, OVS_PACKAGE_VERSION)

const char *get_subprogram_name(void);
void set_subprogram_name(const char *format, ...) OVS_PRINTF_FORMAT(1, 2);
void set_subprogram_name(const char *);

void ovs_print_version(uint8_t min_ofp, uint8_t max_ofp);

Expand Down
3 changes: 1 addition & 2 deletions tests/test-util.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2012, 2013, 2014 Nicira, Inc.
* Copyright (c) 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -497,7 +497,6 @@ test_ovs_scan(struct ovs_cmdl_context *ctx OVS_UNUSED)
long l, l2;
int i, i2;

ovs_assert(ovs_scan("", ""));
ovs_assert(ovs_scan("", " "));
ovs_assert(ovs_scan(" ", " "));
ovs_assert(ovs_scan(" ", " "));
Expand Down

0 comments on commit 40e7cf5

Please sign in to comment.