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

Re: [PATCH] Add confirmation dialog to dlabel code and make one DD dialog less confusing (#669647)



Comments below...

On Thu, 20 Jan 2011, Martin Sivak wrote:

---
loader/driverdisk.c |   10 +++++-----
loader/loader.c     |   38 +++++++++++++++++++++++++++++++++++++-
2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/loader/driverdisk.c b/loader/driverdisk.c
index eb369cb..4b77d03 100644
--- a/loader/driverdisk.c
+++ b/loader/driverdisk.c
@@ -616,11 +616,11 @@ int loadDriverFromMedia(int class, struct loaderData_s *loaderData,
             * them to manually load */
            rc = newtWinTernary(_("Error"), _("Manually choose"),
                                _("Continue"), _("Load another disk"),
-                                _("No devices of the appropriate type were "
-                                  "found on this driver disk.  Would you "
-                                  "like to manually select the driver, "
-                                  "continue anyway, or load another "
-                                  "driver disk?"));
+                                _("No new drivers were found on this driver disk."
+                                  "This may indicate that this disk has already been loaded,"
+                                  "or that the drivers it contains don't match your hardware."
+                                  "Would you like to manually select the driver, "
+                                  "continue anyway, or load another driver disk?"));

            if (rc == 2) {
                /* if they choose to continue, just go ahead and continue */
diff --git a/loader/loader.c b/loader/loader.c
index 74d3e2d..90b2add 100644
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -2115,7 +2115,37 @@ int main(int argc, char ** argv) {
        logMessage(INFO, "Trying to detect vendor driver discs");
        dd = findDriverDiskByLabel();
        dditer = dd;
+
+        /* Start UI for possible confirmation dialog */
+        if (dd && !loaderData.ksFile) {
+            startNewt();
+        }
+
        while(dditer) {
+
+            /* If in interactive mode, ask for confirmation before loading the DD */
+            if (!loaderData.ksFile) {
+                char *buf = NULL;
+                if (asprintf(&buf,
+                             _("Driver disc was detected in %s. "
+                               "Do you want to use it?."),
+                             dditer->data) == -1) {
+                    logMessage(ERROR, "asprintf error in Driver Disc code");
+                    goto ddcontinue;
+                };

checked_asprintf() here instead?  I see you goto ddcontinue rather than
abort after the asprintf() failure, which would be the difference between
this block and checked_asprintf.  But I still think aborting is the proper
course of action if asprintf fails.  Just report the error and die.

+                rc = newtWinChoice(_("Driver disc detected"), _("Use it"), _("Skip it"),
+                                   buf);
+                free(buf);
+                if (rc == 2) {
+                    logMessage(INFO, "Skipping driver disk %s.", (char*)(dditer->data));
+
+                    /* next DD  - bail to the end of the loop for cleanup and continue */
+                    /* and I know I'm using a goto, but it is the same code path as the error above */
+                    goto ddcontinue;
+                }
+            }
+
            /* load the DD */
            if (loadDriverDiskFromPartition(&loaderData, (char*)(dditer->data))) {
                logMessage(ERROR, "Automatic driver disk loader failed for %s.", (char*)(dditer->data));
@@ -2127,7 +2157,8 @@ int main(int argc, char ** argv) {
                mlRestoreModuleState(moduleState);
                busProbe(0);
            }
-
+
+        ddcontinue:
            /* clean the device record */
            free((char*)(dditer->data));
            dditer->data = NULL;
@@ -2136,6 +2167,11 @@ int main(int argc, char ** argv) {
            dditer = g_slist_next(dditer);
        }
        g_slist_free(dd);
+
+        /* Close UI.. */
+        if (dd && !loaderData.ksFile) {
+            stopNewt();
+        }
    }

    if (FL_MODDISK(flags)) {


Looks fine otherwise.

--
David Cantrell <dcantrell redhat com>
Supervisor, Installer Engineering Team
Red Hat, Inc. | Honolulu, HI | UTC-10


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