[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [rhel5-branch] Save bootfile, if we have it, from DHCP response (#448006)



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wed, 1 Jul 2009, Hans de Goede wrote:

Hi,

Comments inline.

On 07/01/2009 05:08 AM, David Cantrell wrote:
Saves the bootfile if we get one so that it can be used later to
construct the default kickstart file to try if the user boots with 'ks'
as a boot parameter.
---
loader2/net.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
  1 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/loader2/net.c b/loader2/net.c
index fbbde60..d25c1aa 100644
--- a/loader2/net.c
+++ b/loader2/net.c
@@ -1532,12 +1532,14 @@ int doDhcp(struct networkDeviceConfig *dev) {
      char *r = NULL, *class = NULL;
      char namebuf[HOST_NAME_MAX];
      time_t timeout;
-    int loglevel, status, ret = 0, i;
+    int loglevel, status, ret = 0, i, sz = 0;
      int shmpump;
      DHCP_Preference pref = 0;
      pid_t pid;
      key_t key;
      int mturet;
+    int culvert[2];
+    char buf[PATH_MAX];

      /* clear existing IP addresses */
      clearInterface(dev->dev.device);
@@ -1602,9 +1604,16 @@ int doDhcp(struct networkDeviceConfig *dev) {

          strncpy(pumpdev->device, dev->dev.device, IF_NAMESIZE);

+        if (pipe(culvert) == -1) {
+ logMessage(ERROR, "%s: pipe(): %s", __func__, strerror(errno));
+            return 1;
+        }
+
          /* call libdhcp in a separate process because libdhcp is bad */
          pid = fork();
          if (pid == 0) {
+            close(culvert[0]);
+
              if (pumpdev->set&  PUMP_INTFINFO_HAS_MTU) {
mturet = nl_set_device_mtu((char *) pumpdev->device, pumpdev->mtu);

@@ -1658,14 +1667,25 @@ int doDhcp(struct networkDeviceConfig *dev) {
                  }
              }

- /* we lose bootFile here, but you know, I just don't care, because - * we don't need it past doDhcp() calls, so whatever --dcantrell
-             */
+            if (pumpdev->set&  PUMP_INTFINFO_HAS_BOOTFILE) {
+                if (pumpdev->bootFile) {
+                    if (write(culvert[1], pumpdev->bootFile,
+                              strlen(pumpdev->bootFile) + 1) == -1) {
+ logMessage(ERROR, "failed to send bootFile to parent "
+                                          "in %s: %s", __func__,
+                                   strerror(errno));
+                    }
+
+                    close(culvert[1]);
+                }
+            }

              exit(0);

culvert[1] should be closed even when there is no bootfile, not that it matters much as we do an exit(0) right afterwards, still the "close(culvert[1]);" should be
outside the if block.

Ooops, right.  Good catch.

          } else if (pid == -1) {
              logMessage(CRITICAL, "dhcp client failed to start");
          } else {
+            close(culvert[1]);
+
              if (waitpid(pid,&status, 0) == -1) {
                  logMessage(ERROR, "waitpid() failure in %s", __func__);
              }
@@ -1733,6 +1753,34 @@ int doDhcp(struct networkDeviceConfig *dev) {
                  }
              }

+            if (dev->dev.set&  PUMP_INTFINFO_HAS_BOOTFILE) {
+                memset(&buf, '\0', sizeof(buf));
+
+                while ((sz = read(culvert[0],&buf, sizeof(buf)))>  0) {
+                    if (dev->dev.bootFile == NULL) {
+                        dev->dev.bootFile = calloc(sizeof(char), sz + 1);
+                        if (dev->dev.bootFile == NULL) {
+                            logMessage(ERROR, "unable to read bootfile");
+                            break;
+                        }
+
+ dev->dev.bootFile = strncpy(dev->dev.bootFile, buf, sz);
+                    } else {
+                        dev->dev.bootFile = realloc(dev->dev.bootFile,
+ strlen(dev->dev.bootFile) +
+                                                    sz + 1);
+                        if (dev->dev.bootFile == NULL) {
+                            logMessage(ERROR, "unable to read bootfile");
+                            break;
+                        }
+
+ dev->dev.bootFile = strncat(dev->dev.bootFile, buf, sz);
+                    }
+                }
+
+                close(culvert[0]);
+            }
+

Same for closing culvert[0] here, that should be outside the if block and here it does
matter.

Also I wonder could this get called twice on the same dev ? It might be a good idea to free (*) dev->dev.bootFile and then set it to NULL before the while, to make sure we are not appending a new bootFile string to an old one, instead of starting a new.

While I'm inclined to say that we won't hit this twice, the way it's currently
written is that bootFile would just grow if we kept hitting the code, so
better safe than less safe.

Updated patch:


- ---
 loader2/net.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/loader2/net.c b/loader2/net.c
index fbbde60..bc4973e 100644
- --- a/loader2/net.c
+++ b/loader2/net.c
@@ -1532,12 +1532,14 @@ int doDhcp(struct networkDeviceConfig *dev) {
     char *r = NULL, *class = NULL;
     char namebuf[HOST_NAME_MAX];
     time_t timeout;
- -    int loglevel, status, ret = 0, i;
+    int loglevel, status, ret = 0, i, sz = 0;
     int shmpump;
     DHCP_Preference pref = 0;
     pid_t pid;
     key_t key;
     int mturet;
+    int culvert[2];
+    char buf[PATH_MAX];

     /* clear existing IP addresses */
     clearInterface(dev->dev.device);
@@ -1602,9 +1604,16 @@ int doDhcp(struct networkDeviceConfig *dev) {

         strncpy(pumpdev->device, dev->dev.device, IF_NAMESIZE);

+        if (pipe(culvert) == -1) {
+            logMessage(ERROR, "%s: pipe(): %s", __func__, strerror(errno));
+            return 1;
+        }
+
         /* call libdhcp in a separate process because libdhcp is bad */
         pid = fork();
         if (pid == 0) {
+            close(culvert[0]);
+
             if (pumpdev->set & PUMP_INTFINFO_HAS_MTU) {
                 mturet = nl_set_device_mtu((char *) pumpdev->device, pumpdev->mtu);

@@ -1658,14 +1667,24 @@ int doDhcp(struct networkDeviceConfig *dev) {
                 }
             }

- -            /* we lose bootFile here, but you know, I just don't care, because
- -             * we don't need it past doDhcp() calls, so whatever   --dcantrell
- -             */
+            if (pumpdev->set & PUMP_INTFINFO_HAS_BOOTFILE) {
+                if (pumpdev->bootFile) {
+                    if (write(culvert[1], pumpdev->bootFile,
+                              strlen(pumpdev->bootFile) + 1) == -1) {
+                        logMessage(ERROR, "failed to send bootFile to parent "
+                                          "in %s: %s", __func__,
+                                   strerror(errno));
+                    }
+                }
+            }

+            close(culvert[1]);
             exit(0);
         } else if (pid == -1) {
             logMessage(CRITICAL, "dhcp client failed to start");
         } else {
+            close(culvert[1]);
+
             if (waitpid(pid, &status, 0) == -1) {
                 logMessage(ERROR, "waitpid() failure in %s", __func__);
             }
@@ -1733,6 +1752,36 @@ int doDhcp(struct networkDeviceConfig *dev) {
                 }
             }

+            if (dev->dev.set & PUMP_INTFINFO_HAS_BOOTFILE) {
+                memset(&buf, '\0', sizeof(buf));
+                free(dev->dev.bootFile);
+                dev->dev.bootFile = NULL;
+
+                while ((sz = read(culvert[0], &buf, sizeof(buf))) > 0) {
+                    if (dev->dev.bootFile == NULL) {
+                        dev->dev.bootFile = calloc(sizeof(char), sz + 1);
+                        if (dev->dev.bootFile == NULL) {
+                            logMessage(ERROR, "unable to read bootfile");
+                            break;
+                        }
+
+                        dev->dev.bootFile = strncpy(dev->dev.bootFile, buf, sz);
+                    } else {
+                        dev->dev.bootFile = realloc(dev->dev.bootFile,
+                                                    strlen(dev->dev.bootFile) +
+                                                    sz + 1);
+                        if (dev->dev.bootFile == NULL) {
+                            logMessage(ERROR, "unable to read bootfile");
+                            break;
+                        }
+
+                        dev->dev.bootFile = strncat(dev->dev.bootFile, buf, sz);
+                    }
+                }
+            }
+
+            close(culvert[0]);
+
             if (shmdt(pumpdev) == -1) {
                 logMessage(ERROR, "%s: shmdt() pumpdev: %s", __func__,
                            strerror(errno));
- -- 1.6.3.3


- -- David Cantrell <dcantrell redhat com>
Red Hat / Honolulu, HI

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAkpLwYgACgkQ5hsjjIy1Vkn2+ACgsL25RU73HJpCTURAqSsWU79g
xb0Anj+P3LbdoL4LOuKnWf5QrdgjrSu2
=Ifl0
-----END PGP SIGNATURE-----


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]