On Jan 8, 2008, at 11:32 PM, Jo Rhett wrote:

What does the following change do? It would seem to change the logic so that resolvedcmd could be null if %s didn't exist in the raw string. Why is this better?

@@ -453,9 +477,8 @@ if (cmd_tail = strstr(rawcmd, "%s"))
   cmd_tail += 2;
   cmd_tail_len = strlen(cmd_tail);
   --cmd_args;
+   strncpy(resolvedcmd, rawcmd, CF_BUFSIZE*2);
   }
-
-strncpy(resolvedcmd, rawcmd, CF_BUFSIZE*2);

Thanks for double-checking. This patch chunk should be scratched. At one point in my working directory I had the following change:

@@ -453,9 +477,15 @@ if (cmd_tail = strstr(rawcmd, "%s"))
    cmd_tail += 2;
    cmd_tail_len = strlen(cmd_tail);
    --cmd_args;
+   strncpy(resolvedcmd, rawcmd, CF_BUFSIZE*2);
+   }
+else
+   {
+   /* Handle "/path/to/installer" (which worked in 2.1.x) */
+   strncpy(resolvedcmd, rawcmd, CF_BUFSIZE*2);
+   if (resolvedcmd[strlen(resolvedcmd)-1] != ' ')
+      strncat(resolvedcmd, " ", 1);
    }
-
-strncpy(resolvedcmd, rawcmd, CF_BUFSIZE*2);

 Verbose("Package manager will be invoked as %s\n", resolvedcmd);
### end patch

This was so that if you just had ``RPMInstallCommand = ( "/usr/sbin/ rpm -i" )'', without the %s, it correctly added a space and then the package list to the command. This was documented functionality that stopped working when the function BuildCommandLine was written, so I thought I would fix the regression. But when I looked in the documentation, there's no longer a reference to being able to leave out the %s, so I thought I wouldn't push this chunk. Unfortunately I only removed the else{} block, not the rest of the change from my published patch.

Good catch.

With regards to whitespace, its just a habit to clean trailing whitespace or to fix the four-space indents that some people have put in when I'm working in the same vicinity, so that my (properly indented) code lines up with the surrounding code (like in the switch block). When I tried submitting my changes in separate patches (including whitespace in its own patch) Mark asked for a single patch. Anyhow, I removed the ws-only blocks, but left the fix for the switch statement.

Updated patch attached.

Attachment: portage.patch
Description: Binary data




--
Eric Searcy
OSU Open Source Lab
_______________________________________________
Bug-cfengine mailing list
[email protected]
https://cfengine.org/mailman/listinfo/bug-cfengine

Reply via email to