Hi,
Comments inline.
On 05/01/2009 11:22 PM, David Cantrell wrote:
In hardware.c, we build a colon-separated list of module names. The
buffer storing that string was set to a hard limit of 1024, which isn't
big enough for certain systems. This patch changes the function to
calculate the length required of the buffer and resize it with
realloc().
---
loader2/hardware.c | 34 ++++++++++++++++++++++++++++------
1 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/loader2/hardware.c b/loader2/hardware.c
index 2cea886..7c574eb 100644
--- a/loader2/hardware.c
+++ b/loader2/hardware.c
@@ -247,9 +247,9 @@ int earlyModuleLoad(moduleInfoSet modInfo, moduleList modLoaded,
int busProbe(moduleInfoSet modInfo, moduleList modLoaded, moduleDeps modDeps,
int justProbe) {
- int i;
+ int i, len = 0;
char ** modList;
- char modules[1024];
+ char *modules = NULL;
/* we always want to try to find out about pcmcia controllers even
* if using noprobe */
Given that modules and len are only used in the "else if (modList) {" block, I think
it would be best to move the declaritions to that block to make this clear.
@@ -269,17 +269,39 @@ int busProbe(moduleInfoSet modInfo, moduleList modLoaded, moduleDeps modDeps,
} else if (modList) {
int probeVirtioAgain = 0;
- *modules = '\0';
-
+ /* compute length of colon-separated string */
+ for (i = 0; modList[i]; i++) {
+ if (i) {
+ len += 1; /* ':' between each module name */
+ }
+
+ len += strlen(modList[i]);
+ }
+
+ len += 1; /* '\0' at the end */
+
Ok.
for (i = 0; modList[i]; i++) {
- if (i) strcat(modules, ":");
- strcat(modules, modList[i]);
+ if (i> 0) {
+ modules = strncat(modules, ":", 1);
+ }
+
+ if (modules == NULL) {
+ modules = strdup(modList[i]);
+
+ if ((modules = realloc(modules, len)) == NULL) {
+ logMessage(ERROR, "error building modules string");
+ return 0;
+ }
+ } else {
+ modules = strncat(modules, modList[i], strlen(modList[i]));
+ }
This seems rather complicated, why first do a strdup and then a realloc ?
why not just do a malloc, followed by a "modules[0] = 0;" and make the strncat
unconditionally?
Also why do the alloc conditionally in the loop? Why not simply do it
unconditionally before the loop?
Last there is no need to use strncat here, a simple strcat will suffice,
actually the strncat is harmfully because we now do not 0 terminate the
string, so if the memory block we got is not zero'd out we may have an
unterminated string.
Regards,
Hans
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
