maint: avoid remaining sprintf uses

* cfg.mk (sc_prohibit_sprintf): New rule.
(sc_prohibit_asprintf): Avoid false positives.
* docs/hacking.html.in (Printf-style functions): Document the
policy.
* HACKING: Regenerate.
* .x-sc_prohibit_sprintf: New exemptions.
* Makefile.am (syntax_check_exceptions): Ship new file.
* src/vbox/vbox_tmpl.c (vboxStartMachine, vboxAttachUSB): Use
virAsprintf instead.
* src/uml/uml_driver.c (umlOpenMonitor): Use snprintf instead.
* tools/virsh.c (cmdDetachInterface): Likewise.
* src/security/security_selinux.c (SELinuxGenSecurityLabel):
Likewise.
* src/openvz/openvz_driver.c (openvzDomainDefineCmd): Likewise,
and ensure large enough buffer.
This commit is contained in:
Eric Blake 2010-08-18 17:31:39 -06:00
parent c811d46fab
commit e8aba782e7
10 changed files with 69 additions and 26 deletions

4
.x-sc_prohibit_sprintf Normal file
View File

@ -0,0 +1,4 @@
^docs/
^po/
^ChangeLog
^HACKING

View File

@ -538,6 +538,12 @@ virAsprintf, in util.h:
This makes it so gcc's -Wformat and -Wformat-security options can do their
jobs and cross-check format strings with the number and types of arguments.
When printing to a string, consider using virBuffer for incremental
allocations, virAsprintf for a one-shot allocation, and snprintf for
fixed-width buffers. Do not use sprintf, even if you can prove the buffer
won't overflow, since gnulib does not provide the same portability guarantees
for sprintf as it does for snprintf.
Use of goto
===========

View File

@ -33,6 +33,7 @@ EXTRA_DIST = \
.x-sc_prohibit_have_config_h \
.x-sc_prohibit_HAVE_MBRTOWC \
.x-sc_prohibit_nonreentrant \
.x-sc_prohibit_sprintf \
.x-sc_prohibit_strcmp \
.x-sc_prohibit_strcmp_and_strncmp \
.x-sc_prohibit_strncpy \

13
cfg.mk
View File

@ -238,10 +238,17 @@ sc_prohibit_strcmp_and_strncmp:
halt='use STREQ() in place of the above uses of str[n]cmp' \
$(_sc_search_regexp)
# Use virAsprintf rather than a'sprintf since *strp is undefined on error.
# Use virAsprintf rather than as'printf since *strp is undefined on error.
sc_prohibit_asprintf:
@prohibit='\<[a]sprintf\>' \
halt='use virAsprintf, not a'sprintf \
@prohibit='\<a[s]printf\>' \
halt='use virAsprintf, not as'printf \
$(_sc_search_regexp)
# Use snprintf rather than s'printf, even if buffer is provably large enough,
# since gnulib has more guarantees for snprintf portability
sc_prohibit_sprintf:
@prohibit='\<[s]printf\>' \
halt='use snprintf, not s'printf \
$(_sc_search_regexp)
sc_prohibit_strncpy:

View File

@ -649,6 +649,15 @@
of arguments.
</p>
<p>
When printing to a string, consider using virBuffer for
incremental allocations, virAsprintf for a one-shot allocation,
and snprintf for fixed-width buffers. Do not use sprintf, even
if you can prove the buffer won't overflow, since gnulib does
not provide the same portability guarantees for sprintf as it
does for snprintf.
</p>
<h2><a name="goto">Use of goto</a></h2>
<p>

View File

@ -58,6 +58,7 @@
#include "memory.h"
#include "bridge.h"
#include "files.h"
#include "intprops.h"
#define VIR_FROM_THIS VIR_FROM_OPENVZ
@ -104,7 +105,7 @@ openvzDomainDefineCmd(const char *args[],
int narg;
int veid;
int max_veid;
char str_id[10];
char str_id[INT_BUFSIZE_BOUND(max_veid)];
FILE *fp;
for (narg = 0; narg < maxarg; narg++)
@ -162,7 +163,7 @@ openvzDomainDefineCmd(const char *args[],
max_veid++;
}
sprintf(str_id, "%d", max_veid);
snprintf(str_id, sizeof(str_id), "%d", max_veid);
ADD_ARG_LIT(str_id);
ADD_ARG_LIT("--name");

View File

@ -182,12 +182,12 @@ SELinuxGenSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED,
c2 = virRandom(1024);
if ( c1 == c2 ) {
sprintf(mcs, "s0:c%d", c1);
snprintf(mcs, sizeof(mcs), "s0:c%d", c1);
} else {
if ( c1 < c2 )
sprintf(mcs, "s0:c%d,c%d", c1, c2);
snprintf(mcs, sizeof(mcs), "s0:c%d,c%d", c1, c2);
else
sprintf(mcs, "s0:c%d,c%d", c2, c1);
snprintf(mcs, sizeof(mcs), "s0:c%d,c%d", c2, c1);
}
} while(mcsAdd(mcs) == -1);

View File

@ -657,7 +657,8 @@ restat:
}
memset(addr.sun_path, 0, sizeof addr.sun_path);
sprintf(addr.sun_path + 1, "libvirt-uml-%u", vm->pid);
snprintf(addr.sun_path + 1, sizeof(addr.sun_path) - 1,
"libvirt-uml-%u", vm->pid);
VIR_DEBUG("Reply address for monitor is '%s'", addr.sun_path+1);
if (bind(priv->monitor, (struct sockaddr *)&addr, sizeof addr) < 0) {
virReportSystemError(errno,

View File

@ -3228,7 +3228,6 @@ vboxStartMachine(virDomainPtr dom, int i, IMachine *machine, vboxIID *iid)
PRUnichar *valueDisplayUtf16 = NULL;
char *valueDisplayUtf8 = NULL;
IProgress *progress = NULL;
char displayutf8[32] = {0};
PRUnichar *env = NULL;
PRUnichar *sessionType = NULL;
nsresult rc;
@ -3308,8 +3307,13 @@ vboxStartMachine(virDomainPtr dom, int i, IMachine *machine, vboxIID *iid)
if (guiPresent) {
if (guiDisplay) {
sprintf(displayutf8, "DISPLAY=%.24s", guiDisplay);
VBOX_UTF8_TO_UTF16(displayutf8, &env);
char *displayutf8;
if (virAsprintf(&displayutf8, "DISPLAY=%s", guiDisplay) < 0)
virReportOOMError();
else {
VBOX_UTF8_TO_UTF16(displayutf8, &env);
VIR_FREE(displayutf8);
}
VIR_FREE(guiDisplay);
}
@ -3318,8 +3322,13 @@ vboxStartMachine(virDomainPtr dom, int i, IMachine *machine, vboxIID *iid)
if (sdlPresent) {
if (sdlDisplay) {
sprintf(displayutf8, "DISPLAY=%.24s", sdlDisplay);
VBOX_UTF8_TO_UTF16(displayutf8, &env);
char *displayutf8;
if (virAsprintf(&displayutf8, "DISPLAY=%s", sdlDisplay) < 0)
virReportOOMError();
else {
VBOX_UTF8_TO_UTF16(displayutf8, &env);
VIR_FREE(displayutf8);
}
VIR_FREE(sdlDisplay);
}
@ -4526,19 +4535,22 @@ vboxAttachUSB(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
if (def->hostdevs[i]->source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
char filtername[11] = {0};
char *filtername = NULL;
PRUnichar *filternameUtf16 = NULL;
IUSBDeviceFilter *filter = NULL;
/* Assuming can't have more then 9999 devices so
* restricting to %04d
/* Zero pad for nice alignment when fewer than 9999
* devices.
*/
sprintf(filtername, "filter%04d", i);
VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
USBController->vtbl->CreateDeviceFilter(USBController,
filternameUtf16,
&filter);
if (virAsprintf(&filtername, "filter%04d", i) < 0) {
virReportOOMError();
} else {
VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
VIR_FREE(filtername);
USBController->vtbl->CreateDeviceFilter(USBController,
filternameUtf16,
&filter);
}
VBOX_UTF16_FREE(filternameUtf16);
if (filter &&
@ -4551,13 +4563,15 @@ vboxAttachUSB(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
char productId[40] = {0};
if (def->hostdevs[i]->source.subsys.u.usb.vendor) {
sprintf(vendorId, "%x", def->hostdevs[i]->source.subsys.u.usb.vendor);
snprintf(vendorId, sizeof(vendorId), "%x",
def->hostdevs[i]->source.subsys.u.usb.vendor);
VBOX_UTF8_TO_UTF16(vendorId, &vendorIdUtf16);
filter->vtbl->SetVendorId(filter, vendorIdUtf16);
VBOX_UTF16_FREE(vendorIdUtf16);
}
if (def->hostdevs[i]->source.subsys.u.usb.product) {
sprintf(productId, "%x", def->hostdevs[i]->source.subsys.u.usb.product);
snprintf(productId, sizeof(productId), "%x",
def->hostdevs[i]->source.subsys.u.usb.product);
VBOX_UTF8_TO_UTF16(productId, &productIdUtf16);
filter->vtbl->SetProductId(filter,
productIdUtf16);

View File

@ -8468,7 +8468,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
sprintf(buf, "/domain/devices/interface[@type='%s']", type);
snprintf(buf, sizeof(buf), "/domain/devices/interface[@type='%s']", type);
obj = xmlXPathEval(BAD_CAST buf, ctxt);
if ((obj == NULL) || (obj->type != XPATH_NODESET) ||
(obj->nodesetval == NULL) || (obj->nodesetval->nodeNr == 0)) {