[PATCH 4/6 v2] Convert malloc+sprintf pairs into X*asprintf() calls

Alan Coopersmith alan.coopersmith at oracle.com
Tue Nov 30 21:53:29 PST 2010


Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
Reviewed-by: Mikhail Gusarov <dottedmag at dottedmag.net>
Reviewed-by: Julien Cristau <jcristau at debian.org>
---
 config/hal.c                      |    9 +++------
 hw/xfree86/common/xf86Configure.c |   33 +++++++++++----------------------
 hw/xfree86/common/xf86Helper.c    |    5 +----
 hw/xfree86/common/xf86Init.c      |    3 +--
 hw/xfree86/common/xf86ShowOpts.c  |    3 +++
 hw/xfree86/common/xf86pciBus.c    |    4 ++--
 hw/xfree86/common/xf86sbusBus.c   |    6 ++----
 hw/xfree86/dri/dri.c              |    7 ++-----
 hw/xfree86/loader/loadmod.c       |   12 ++++++------
 hw/xfree86/vbe/vbeModes.c         |    4 ++--
 os/log.c                          |   12 +++---------
 11 files changed, 36 insertions(+), 62 deletions(-)

diff --git a/config/hal.c b/config/hal.c
index cd16f8b..d0a4564 100644
--- a/config/hal.c
+++ b/config/hal.c
@@ -63,10 +63,8 @@ device_removed(LibHalContext *ctx, const char *udi)
 {
     char *value;
 
-    value = malloc(strlen(udi) + 5); /* "hal:" + NULL */
-    if (!value)
+    if (Xasprintf (&value, "hal:%s", udi) == -1)
         return;
-    sprintf(value, "hal:%s", udi);
 
     remove_devices("hal", value);
 
@@ -228,12 +226,11 @@ device_added(LibHalContext *hal_ctx, const char *udi)
     add_option(&options, "driver", driver);
     add_option(&options, "name", name);
 
-    config_info = malloc(strlen(udi) + 5); /* "hal:" and NULL */
-    if (!config_info) {
+    if (Xasprintf (&config_info, "hal:%s", udi) == -1) {
+        config_info = NULL;
         LogMessage(X_ERROR, "config/hal: couldn't allocate name\n");
         goto unwind;
     }
-    sprintf(config_info, "hal:%s", udi);
 
     /* Check for duplicate devices */
     if (device_is_duplicate(config_info))
diff --git a/hw/xfree86/common/xf86Configure.c b/hw/xfree86/common/xf86Configure.c
index 7235c61..48acc8e 100644
--- a/hw/xfree86/common/xf86Configure.c
+++ b/hw/xfree86/common/xf86Configure.c
@@ -205,12 +205,9 @@ configureScreenSection (int screennum)
     int depths[] = { 1, 4, 8, 15, 16, 24/*, 32*/ };
     parsePrologue (XF86ConfScreenPtr, XF86ConfScreenRec)
 
-    ptr->scrn_identifier = malloc(18);
-    sprintf(ptr->scrn_identifier, "Screen%d", screennum);
-    ptr->scrn_monitor_str = malloc(19);
-    sprintf(ptr->scrn_monitor_str, "Monitor%d", screennum);
-    ptr->scrn_device_str = malloc(16);
-    sprintf(ptr->scrn_device_str, "Card%d", screennum);
+    XNFasprintf(&ptr->scrn_identifier, "Screen%d", screennum);
+    XNFasprintf(&ptr->scrn_monitor_str, "Monitor%d", screennum);
+    XNFasprintf(&ptr->scrn_device_str, "Card%d", screennum);
 
     for (i=0; i<sizeof(depths)/sizeof(depths[0]); i++)
     {
@@ -256,14 +253,13 @@ optionTypeToString(OptionValueType type)
 static XF86ConfDevicePtr
 configureDeviceSection (int screennum)
 {
-    char identifier[16];
     OptionInfoPtr p;
     int i = 0;
     parsePrologue (XF86ConfDevicePtr, XF86ConfDeviceRec)
 
     /* Move device info to parser structure */
-    sprintf(identifier, "Card%d", screennum);
-    ptr->dev_identifier = strdup(identifier);
+    if (Xasprintf(&ptr->dev_identifier, "Card%d", screennum) == -1)
+        ptr->dev_identifier = NULL;
     ptr->dev_chipset = DevToConfig[screennum].GDev.chipset;
     ptr->dev_busid = DevToConfig[screennum].GDev.busID;
     ptr->dev_driver = DevToConfig[screennum].GDev.driver;
@@ -306,10 +302,8 @@ configureDeviceSection (int screennum)
 		int len = strlen(ptr->dev_comment) + strlen(prefix) +
 			  strlen(middle) + strlen(suffix) + 1;
 		
-		optname = malloc(strlen(p->name) + 2 + 1);
-		if (!optname)
+		if (Xasprintf(&optname, "\"%s\"", p->name) == -1)
 		    break;
-		sprintf(optname, "\"%s\"", p->name);
 
 		len += max(20, strlen(optname));
 		len += strlen(opttype);
@@ -370,16 +364,14 @@ configureLayoutSection (void)
 	aptr->adj_x = 0;
 	aptr->adj_y = 0;
 	aptr->adj_scrnum = scrnum;
-	aptr->adj_screen_str = xnfalloc(18);
-	sprintf(aptr->adj_screen_str, "Screen%d", scrnum);
+	XNFasprintf(&aptr->adj_screen_str, "Screen%d", scrnum);
 	if (scrnum == 0) {
 	    aptr->adj_where = CONF_ADJ_ABSOLUTE;
 	    aptr->adj_refscreen = NULL;
 	}
 	else {
 	    aptr->adj_where = CONF_ADJ_RIGHTOF;
-	    aptr->adj_refscreen = xnfalloc(18);
-	    sprintf(aptr->adj_refscreen, "Screen%d", scrnum - 1);
+	    XNFasprintf(&aptr->adj_refscreen, "Screen%d", scrnum - 1);
 	}
     	ptr->lay_adjacency_lst =
 	    (XF86ConfAdjacencyPtr)xf86addListItem((glp)ptr->lay_adjacency_lst,
@@ -443,8 +435,7 @@ configureMonitorSection (int screennum)
 {
     parsePrologue (XF86ConfMonitorPtr, XF86ConfMonitorRec)
 
-    ptr->mon_identifier = malloc(19);
-    sprintf(ptr->mon_identifier, "Monitor%d", screennum);
+    XNFasprintf(&ptr->mon_identifier, "Monitor%d", screennum);
     ptr->mon_vendor = strdup("Monitor Vendor");
     ptr->mon_modelname = strdup("Monitor Model");
 
@@ -491,11 +482,9 @@ configureDDCMonitorSection (int screennum)
 
     parsePrologue (XF86ConfMonitorPtr, XF86ConfMonitorRec)
 
-    ptr->mon_identifier = malloc(19);
-    sprintf(ptr->mon_identifier, "Monitor%d", screennum);
+    XNFasprintf(&ptr->mon_identifier, "Monitor%d", screennum);
     ptr->mon_vendor = strdup(ConfiguredMonitor->vendor.name);
-    ptr->mon_modelname = malloc(12);
-    sprintf(ptr->mon_modelname, "%x", ConfiguredMonitor->vendor.prod_id);
+    XNFasprintf(&ptr->mon_modelname, "%x", ConfiguredMonitor->vendor.prod_id);
 
     /* features in centimetres, we want millimetres */
     mon_width  = 10 * ConfiguredMonitor->features.hsize ;
diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c
index a28cc99..efcd8f4 100644
--- a/hw/xfree86/common/xf86Helper.c
+++ b/hw/xfree86/common/xf86Helper.c
@@ -1280,11 +1280,8 @@ xf86LogInit(void)
     /* Get the log file name */
     if (xf86LogFileFrom == X_DEFAULT) {
 	/* Append the display number and ".log" */
-	lf = malloc(strlen(xf86LogFile) + strlen("%s") +
-		    strlen(LOGSUFFIX) + 1);
-	if (!lf)
+	if (Xasprintf(&lf, "%s%%s" LOGSUFFIX, xf86LogFile) == -1)
 	    FatalError("Cannot allocate space for the log file name\n");
-	sprintf(lf, "%s%%s" LOGSUFFIX, xf86LogFile);
 	xf86LogFile = lf;
     }
 
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index ef90fa5..78f51e1 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -536,8 +536,7 @@ InitOutput(ScreenInfo *pScreenInfo, int argc, char **argv)
 
     for (i = 0; i < xf86NumScreens; i++) {
       if (xf86Screens[i]->name == NULL) {
-	xf86Screens[i]->name = xnfalloc(strlen("screen") + 10 + 1);
-	sprintf(xf86Screens[i]->name, "screen%d", i);
+	XNFasprintf(&xf86Screens[i]->name, "screen%d", i);
 	xf86MsgVerb(X_WARNING, 0,
 		    "Screen driver %d has no name set, using `%s'.\n",
 		    i, xf86Screens[i]->name);
diff --git a/hw/xfree86/common/xf86ShowOpts.c b/hw/xfree86/common/xf86ShowOpts.c
index eac25d7..ce86090 100644
--- a/hw/xfree86/common/xf86ShowOpts.c
+++ b/hw/xfree86/common/xf86ShowOpts.c
@@ -111,6 +111,9 @@ void DoShowOptions (void) {
 				);
 				for (p = pOption; p->name != NULL; p++) {
 					const char *opttype = optionTypeToSting(p->type);
+					/* XXX: Why overallocate by 2 bytes?
+					 * Otherwise, this would be strdup()
+					 */
 					char *optname = malloc(strlen(p->name) + 2 + 1);
 					if (!optname) {
 						continue;                      
diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c
index d689832..447b192 100644
--- a/hw/xfree86/common/xf86pciBus.c
+++ b/hw/xfree86/common/xf86pciBus.c
@@ -1347,9 +1347,9 @@ xf86PciConfigureNewDev(void *busData, struct pci_device *pVideo,
 
     pVideo = (struct pci_device *) busData;
 
-    GDev->busID = xnfalloc(16);
     xf86FormatPciBusNumber(pVideo->bus, busnum);
-    sprintf(GDev->busID, "PCI:%s:%d:%d", busnum, pVideo->dev, pVideo->func);
+    XNFasprintf(&GDev->busID, "PCI:%s:%d:%d",
+		busnum, pVideo->dev, pVideo->func);
 
     GDev->chipID = pVideo->device_id;
     GDev->chipRev = pVideo->revision;
diff --git a/hw/xfree86/common/xf86sbusBus.c b/hw/xfree86/common/xf86sbusBus.c
index d7c928b..8cfac84 100644
--- a/hw/xfree86/common/xf86sbusBus.c
+++ b/hw/xfree86/common/xf86sbusBus.c
@@ -706,11 +706,9 @@ xf86SbusConfigureNewDev(void *busData, sbusDevicePtr sBus, GDevRec *GDev)
         sparcPromClose();
     }
     if (promPath) {
-        GDev->busID = xnfalloc(strlen(promPath) + 6);
-        sprintf(GDev->busID, "SBUS:%s", promPath);
+        XNFasprintf(&GDev->busID, "SBUS:%s", promPath);
         free(promPath);
     } else {
-        GDev->busID = xnfalloc(12);
-        sprintf(GDev->busID, "SBUS:fb%d", sBus->fbNum);
+        XNFsprintf(&GDev->busID, "SBUS:fb%d", sBus->fbNum);
     }
 }
diff --git a/hw/xfree86/dri/dri.c b/hw/xfree86/dri/dri.c
index fe99a2d..6dd97ae 100644
--- a/hw/xfree86/dri/dri.c
+++ b/hw/xfree86/dri/dri.c
@@ -2426,13 +2426,10 @@ DRICreatePCIBusID(const struct pci_device * dev)
 {
     char *busID;
 
-    busID = malloc(20);
-    if (busID == NULL)
+    if (Xasprintf(&busID, "pci:%04x:%02x:%02x.%d", dev->domain, dev->bus,
+		  dev->dev, dev->func) == -1)
 	return NULL;
 
-    snprintf(busID, 20, "pci:%04x:%02x:%02x.%d", dev->domain, dev->bus,
-	dev->dev, dev->func);
-
     return busID;
 }
 
diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 06d082b..751fec6 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -406,22 +406,22 @@ FindModuleInSubdir(const char *dirpath, const char *module)
  
         snprintf(tmpBuf, PATH_MAX, "lib%s.so", module);
         if (strcmp(direntry->d_name, tmpBuf) == 0) {
-            ret = malloc(strlen(tmpBuf) + strlen(dirpath) + 1);
-            sprintf(ret, "%s%s", dirpath, tmpBuf);
+            if (Xasprintf(&ret, "%s%s", dirpath, tmpBuf) == -1)
+		ret = NULL;
             break;
         }
 
         snprintf(tmpBuf, PATH_MAX, "%s_drv.so", module);
         if (strcmp(direntry->d_name, tmpBuf) == 0) {
-            ret = malloc(strlen(tmpBuf) + strlen(dirpath) + 1);
-            sprintf(ret, "%s%s", dirpath, tmpBuf);
+            if (Xasprintf(&ret, "%s%s", dirpath, tmpBuf) == -1)
+		ret = NULL;
             break;
         }
 
         snprintf(tmpBuf, PATH_MAX, "%s.so", module);
         if (strcmp(direntry->d_name, tmpBuf) == 0) {
-            ret = malloc(strlen(tmpBuf) + strlen(dirpath) + 1);
-            sprintf(ret, "%s%s", dirpath, tmpBuf);
+            if (Xasprintf(&ret, "%s%s", dirpath, tmpBuf) == -1)
+		ret = NULL;
             break;
         }
     }
diff --git a/hw/xfree86/vbe/vbeModes.c b/hw/xfree86/vbe/vbeModes.c
index 3f2cae5..ea24b61 100644
--- a/hw/xfree86/vbe/vbeModes.c
+++ b/hw/xfree86/vbe/vbeModes.c
@@ -356,8 +356,8 @@ VBESetModeNames(DisplayModePtr pMode)
 		pMode->VDisplay > 10000 || pMode->VDisplay < 0) {
 		pMode->name = strdup("BADMODE");
 	    } else {
-		pMode->name = xnfalloc(4 + 1 + 4 + 1);
-		sprintf(pMode->name, "%dx%d", pMode->HDisplay, pMode->VDisplay);
+		XNFasprintf(&pMode->name, "%dx%d",
+			    pMode->HDisplay, pMode->VDisplay);
 	    }
 	}
 	pMode = pMode->next;
diff --git a/os/log.c b/os/log.c
index fdcf91c..881bcd1 100644
--- a/os/log.c
+++ b/os/log.c
@@ -177,10 +177,8 @@ LogInit(const char *fname, const char *backup)
     char *logFileName = NULL;
 
     if (fname && *fname) {
-	logFileName = malloc(strlen(fname) + strlen(display) + 1);
-	if (!logFileName)
+	if (Xasprintf(&logFileName, fname, display) == -1)
 	    FatalError("Cannot allocate space for the log file name\n");
-	sprintf(logFileName, fname, display);
 
 	if (backup && *backup) {
 	    struct stat buf;
@@ -189,13 +187,9 @@ LogInit(const char *fname, const char *backup)
 		char *suffix;
 		char *oldLog;
 
-		oldLog = malloc(strlen(logFileName) + strlen(backup) +
-				strlen(display) + 1);
-		suffix = malloc(strlen(backup) + strlen(display) + 1);
-		if (!oldLog || !suffix)
+		if ((Xasprintf(&suffix, backup, display) == -1) ||
+		    (Xasprintf(&oldLog, "%s%s", logFileName, suffix) == -1))
 		    FatalError("Cannot allocate space for the log file name\n");
-		sprintf(suffix, backup, display);
-		sprintf(oldLog, "%s%s", logFileName, suffix);
 		free(suffix);
 		if (rename(logFileName, oldLog) == -1) {
 		    FatalError("Cannot move old log file \"%s\" to \"%s\"\n",
-- 
1.7.3.2



More information about the xorg-devel mailing list