Well the bugnumbers.. all those patches fix pieces and bits from the following list of bugs (all product and releases combined and relevant for this work): 466754, 469709, 485060, 489314, 500743, 504155, 508242, 515437, 522400, 523666, 538590, + couple of bugs closed with RHEL5.3 and RHEL5.4 I asked for comments for the feature mentioned in 538591.. and I ment it.. so far no response We are not doing 538597, rpm does this just fine Also bear in mind, that most of the functionality IS present and was reviewed in RHEL5. The algorithm description was described in the leading email. The goals are in the documentation I added in another patch. Martin ----- "David Cantrell" <dcantrell@xxxxxxxxxx> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Apologies for duplicate comments, I'm just running through > everything. > > On Wed, 2 Dec 2009, Martin Sivak wrote: > > > - updates the code to agree with current anaconda API > > - adds dlabel functionality > > - also adds some stub calls to set modprobe behaviour > > This comment improves things, but I'd really like to see more. A > patch like > this with 355 insertions deserves more of a writeup. I would like to > see a > detailed comment starting with a short one line description > (preferably <= > 76 chars) including the bug number, then paragraphs explaining what > the patch > is doing, what was changed, etc. > > Since multiple files were changed and multiple functions touched (and > new > ones added), I'd prefer to see explanations for that. > > > --- > > loader/driverdisk.c | 333 > +++++++++++++++++++++++++++++++++++++++++++++------ > > loader/driverdisk.h | 20 +++ > > loader/loader.c | 37 ++++++ > > loader/loader.h | 2 + > > 4 files changed, 355 insertions(+), 37 deletions(-) > > > > diff --git a/loader/driverdisk.c b/loader/driverdisk.c > > index 5f8d43c..f718684 100644 > > --- a/loader/driverdisk.c > > +++ b/loader/driverdisk.c > > @@ -30,6 +30,12 @@ > > #include <unistd.h> > > #include <glib.h> > > > > +#include <blkid/blkid.h> > > + > > +#include <glob.h> > > +#include <rpm/rpmlib.h> > > +#include <sys/utsname.h> > > + > > #include "copy.h" > > #include "loader.h" > > #include "log.h" > > @@ -48,6 +54,8 @@ > > #include "nfsinstall.h" > > #include "urlinstall.h" > > > > +#include "rpmextract.h" > > + > > #include "../isys/isys.h" > > #include "../isys/imount.h" > > #include "../isys/eddsupport.h" > > @@ -55,37 +63,175 @@ > > /* boot flags */ > > extern uint64_t flags; > > > > -static char * driverDiskFiles[] = { "modinfo", "modules.dep", > > - "modules.cgz", "modules.alias", > NULL }; > > +/* modprobe DD mode */ > > +int modprobeDDmode() > > +{ > > + FILE *f = fopen("/etc/depmod.d/ddmode.conf", "w"); > > + if(f){ > > + struct utsname unamedata; > > + if(uname(&unamedata)){ > > + fprintf(f, " pblacklist /lib/modules\n"); > > + } > > + else{ > > + fprintf(f, " pblacklist /lib/modules/%s\n", > unamedata.release); > > + } > > + > > + fclose(f); > > + } > > + > > + return f==NULL; > > +} > > I think Peter commented on the coding style. The issues here that I > have a > problem with are indentation, spacing, and the function declaration. > We > generally use 4 space indents, you've done 2. We also do stuff like: > > if (f) { > > and not: > > if(f){ > > For the function declaration, I don't like functionName(). I'd > prefer > functionName(void). > > These are nits, I know, but this is all new code and I would much > prefer that > we maintain coding style consistency. A good guide is here: > > http://www.x.org/wiki/CodingStyle > > I'd say with the exception of function naming, we do most of the other > stuff. > I'm ok with function_names_like_this or functionNamesLikeThis. > > > +int modprobeNormalmode() > > +{ > > + unlink("/etc/depmod.d/ddmode.conf"); > > + > > + /* run depmod to refresh modules db */ > > + if(system("depmod -a")){ > > + /* FIXME: depmod didn't run */ > > + } > > + > > + return 0; > > +} > > If unlink() fails or system() fails, that would be nice to know and > why. > > > +/* RPM extraction dependency checks */ > > +int dlabelDeps(const char* depends, void *userptr) > > +{ > > + logMessage(DEBUGLVL, "Depends on: %s\n", depends); > > + return 0; > > +} > > I assume this is a stub and will contain more later? > > > +int dlabelProvides(const char* dep, void *userptr) > > +{ > > + char *kernelver = (char*)userptr; > > + > > + logMessage(DEBUGLVL, "Provides: %s\n", dep); > > + > > + return strcmp(dep, kernelver); > > +} > > Will this contain more at some point? > > > +int dlabelFilter(const char* name, struct stat *fstat, void > *userptr) > > +{ > > + int l = strlen(name); > > + > > + logMessage(DEBUGLVL, "Unpacking %s (%lluB)\n", name, > fstat->st_size); > > + > > + /* we want firmware files */ > > + if(!strncmp("lib/firmware/", name, 13)) return 0; > > + > > + if(l<3) return 1; > > + l-=3; > > + > > + /* and we want only .ko files */ > > + if(strncmp(".ko", name+l, 3)) return 1; > > + > > + /* TODO we are unpacking kernel module, read it's description */ > > + > > + return 0; > > +} > > + > > +char* moduleDescription(const char* modulePath) > > +{ > > + char *command = rstrscat(NULL, "modinfo --description '", > modulePath, "'", NULL); > > + FILE *f = popen(command, "r"); > > + if(f==NULL) return NULL; > > + > > + char *description = malloc(sizeof(char)*256); > > + int size = fread(description, 1, 255, f); > > + if(size==0){ > > + free(description); > > + return NULL; > > + } > > + description[size-1]=0; /* strip the trailing newline */ > > + pclose(f); > > + > > + free(command); > > + return description; > > +} > > Rather than the malloc() and fread() here, I'd like to see us make use > of more > convenience functions from glib because we don't then have to deal > with our own > memory management or static buffers. The g_spawn_sync() function > would work > well here. It can run modinfo and capture stdout and stderr. Here is > a > verbose standalone example showing how to get the module description > via > modinfo with glib functions (this is also meant to demonstrate some of > the > other stuff that glib can do): > > - ------ > /* > * use modinfo to get kernel module description, report errors > * overly verbose example demonstrating glib convenience functions > */ > > /* gcc -O0 -g -Wall $(pkg-config --cflags glib-2.0) getModDesc.c \ > * $(pkg-config --libs glib-2.0) > */ > > /* dcantrell@xxxxxxxxxx */ > > #include <stdio.h> > #include <stdlib.h> > #include <glib.h> > > int main(int argc, char **argv) { > GString *tmp = g_string_new("modinfo -F description "); > gchar **modinfoArgs = NULL; > GSpawnFlags flags = G_SPAWN_SEARCH_PATH; > gchar *output = NULL; > gchar *errors = NULL; > gint status = 0; > GError *e = NULL; > > if (argc != 2) { > fprintf(stderr, "Please specify one module file by full > path.\n"); > return EXIT_FAILURE; > } > > /* append the module path to our command, split in to argument > vector */ > g_string_append_printf(tmp, argv[1]); > modinfoArgs = g_strsplit(tmp->str, " ", 0); > g_string_free(tmp, TRUE); > > if (!modinfoArgs) { > fprintf(stderr, "modinfoArgs split failure\n"); > return EXIT_FAILURE; > } > > /* run modinfo, get description, report errors on failure */ > if (!g_spawn_sync(NULL, modinfoArgs, NULL, flags, NULL, NULL, > &output, &errors, &status, &e)) { > fprintf(stderr, "Error getting module description: %s\n", > e->message); > g_error_free(e); > > if (errors) { > fprintf(stderr, "modinfo error: %s\n", errors); > } > > return EXIT_FAILURE; > } > > /* display the module description */ > if (output) { > /* we do not want trailing newlines */ > output = g_strchomp(output); > > printf("%s\n", output); > } > > return EXIT_SUCCESS; > } > - ------ > > I would like to avoid static buffers when we can. > > > +int globErrFunc(const char *epath, int eerrno) > > +{ > > + return 0; > > +} > > I see you're passing this to glob to toss errors, but according to the > glob(3) > man page, you could pass NULL to glob() and get the same effect. > > > +int dlabelUnpackRPMDir(char* rpmdir, char* destination) > > +{ > > + char *kernelver; > > + struct utsname unamedata; > > + char *oldcwd; > > + char *globpattern; > > + int rc; > > + > > + /* get current working directory */ > > + oldcwd = getcwd(NULL, 0); > > + > > + /* set the cwd to destination */ > > + if(chdir(destination)){ > > + free(oldcwd); > > + return 1; > > + } > > + > > + /* get running kernel version */ > > + if(uname(&unamedata)){ > > + kernelver = rstrscat(NULL, "kernel-modules-", "unknown", > NULL); > > + } > > + else{ > > + kernelver = rstrscat(NULL, "kernel-modules-", > unamedata.release, NULL); > > + } > > + logMessage(DEBUGLVL, "Kernel version: %s\n", kernelver); > > + > > + globpattern = rstrscat(NULL, rpmdir, "/*.rpm", NULL); > > + glob_t globres; > > + char** globitem; > > + if(!glob(globpattern, GLOB_NOSORT|GLOB_NOESCAPE, globErrFunc, > &globres)){ > > + /* iterate over all rpm files */ > > + globitem = globres.gl_pathv; > > + while(globres.gl_pathc>0 && globitem!=NULL){ > > + explodeRPM(*globitem, dlabelFilter, dlabelProvides, > dlabelDeps, kernelver); > > + } > > + globfree(&globres); > > + /* end of iteration */ > > + } > > + free(globpattern); > > + > > + /* restore CWD */ > > + if(chdir(oldcwd)){ > > + /* FIXME: too bad.. */ > > + } > > + > > + /* cleanup */ > > + free(kernelver); > > + free(oldcwd); > > + return rc; > > +} > > See email reply to your first message in this thread for my rpm > handling > comments. > > > +static char * driverDiskFiles[] = { "repodata", NULL }; > > > > static int verifyDriverDisk(char *mntpt) { > > char ** fnPtr; > > char file[200]; > > struct stat sb; > > > > - for (fnPtr = driverDiskFiles; *fnPtr; fnPtr++) { > > - sprintf(file, "%s/%s", mntpt, *fnPtr); > > - if (access(file, R_OK)) { > > - logMessage(ERROR, "cannot find %s, bad driver disk", > file); > > - return LOADER_BACK; > > - } > > - } > > - > > - /* check for both versions */ > > + /* check for dd descriptor */ > > sprintf(file, "%s/rhdd", mntpt); > > if (access(file, R_OK)) { > > - logMessage(DEBUGLVL, "not a new format driver disk, > checking for old"); > > - sprintf(file, "%s/rhdd-6.1", mntpt); > > - if (access(file, R_OK)) { > > - logMessage(ERROR, "can't find either driver disk > identifier, bad " > > - "driver disk"); > > - } > > + logMessage(ERROR, "can't find driver disk identifier, bad > " > > + "driver disk"); > > + return LOADER_BACK; > > } > > > > /* side effect: file is still mntpt/ddident */ > > stat(file, &sb); > > if (!sb.st_size) > > return LOADER_BACK; > > + for (fnPtr = driverDiskFiles; *fnPtr; fnPtr++) { > > + sprintf(file, "%s/%s/%s", mntpt, getProductArch(), > *fnPtr); > > + if (access(file, R_OK)) { > > + logMessage(ERROR, "cannot find %s, bad driver disk", > file); > > + return LOADER_BACK; > > + } > > + } > > > > return LOADER_OK; > > } > > @@ -101,25 +247,20 @@ static void copyErrorFn (char *msg) { > > /* this copies the contents of the driver disk to a ramdisk and > loads > > * the moduleinfo, etc. assumes a "valid" driver disk mounted at > mntpt */ > > static int loadDriverDisk(struct loaderData_s *loaderData, char > *mntpt) { > > - moduleInfoSet modInfo = loaderData->modInfo; > > - char file[200], dest[200]; > > + /* FIXME moduleInfoSet modInfo = loaderData->modInfo; */ > > + char file[200], dest[200], src[200]; > > char *title; > > char *fwdir = NULL; > > struct moduleBallLocation * location; > > struct stat sb; > > static int disknum = 0; > > - int version = 1; > > int fd, ret; > > > > - /* check for both versions */ > > - sprintf(file, "%s/rhdd", mntpt); > > + /* check for new version */ > > + sprintf(file, "%s/rhdd3", mntpt); > > if (access(file, R_OK)) { > > - version = 0; > > - sprintf(file, "%s/rhdd-6.1", mntpt); > > - if (access(file, R_OK)) { > > - /* this can't happen, we already verified it! */ > > - return LOADER_BACK; > > - } > > + /* this can't happen, we already verified it! */ > > + return LOADER_BACK; > > } > > stat(file, &sb); > > title = malloc(sb.st_size + 1); > > @@ -131,23 +272,35 @@ static int loadDriverDisk(struct loaderData_s > *loaderData, char *mntpt) { > > title[sb.st_size] = '\0'; > > close(fd); > > > > - sprintf(file, "/tmp/DD-%d", disknum); > > + sprintf(file, DD_RPMS, disknum); > > mkdirChain(file); > > + mkdirChain(DD_MODULES); > > + mkdirChain(DD_FIRMWARE); > > > > if (!FL_CMDLINE(flags)) { > > startNewt(); > > winStatus(40, 3, _("Loading"), _("Reading driver disk")); > > } > > > > - sprintf(dest, "/tmp/DD-%d", disknum); > > - copyDirectory(mntpt, dest, copyWarnFn, copyErrorFn); > > - > > location = malloc(sizeof(struct moduleBallLocation)); > > location->title = strdup(title); > > - location->version = version; > > > > - checked_asprintf(&location->path, "/tmp/DD-%d/modules.cgz", > disknum); > > - checked_asprintf(&fwdir, "/tmp/DD-%d/firmware", disknum); > > + checked_asprintf(&location->path, DD_MODULES); > > + checked_asprintf(&fwdir, DD_FIRMWARE); > > + > > + sprintf(dest, DD_RPMS, disknum); > > + sprintf(src, "%s/rpms/%s", mntpt, getProductArch()); > > + copyDirectory(src, dest, copyWarnFn, copyErrorFn); > > + > > + /* unpack packages from dest into location->path */ > > + if(dlabelUnpackRPMDir(dest, DD_EXTRACTED)){ > > + /* TODO error handler */ > > + } > > + > > + /* run depmod to refresh modules db */ > > + if(system("depmod -a")){ > > + /* FIXME: depmod didn't run */ > > + } > > > > if (!access(fwdir, R_OK|X_OK)) { > > add_fw_search_dir(loaderData, fwdir); > > @@ -156,8 +309,11 @@ static int loadDriverDisk(struct loaderData_s > *loaderData, char *mntpt) { > > } > > free(fwdir); > > > > - sprintf(file, "%s/modinfo", mntpt); > > - readModuleInfo(file, modInfo, location, 1); > > + /* TODO generate and read module info > > + * > > + * sprintf(file, "%s/modinfo", mntpt); > > + * readModuleInfo(file, modInfo, location, 1); > > + */ > > > > if (!FL_CMDLINE(flags)) > > newtPopWindow(); > > @@ -623,3 +779,106 @@ static void getDDFromDev(struct loaderData_s * > loaderData, char * dev) { > > umount("/tmp/drivers"); > > unlink("/tmp/drivers"); > > } > > Given the opportunity, I'd like to see older code using static buffers > and > sprintf() replaced with checked_asprintf(). > > > + > > +/* > > + * Utility functions to maintain linked-list of device names > > + * */ > > + > > +struct ddlist* ddlist_add(struct ddlist *list, const char* device) > > +{ > > + struct ddlist* item; > > + > > + item = (struct ddlist*)malloc(sizeof(struct ddlist)); > > + if(item==NULL){ > > + return list; > > + } > > + > > + item->device = strdup(device); > > + item->next = list; > > + > > + return item; > > +} > > + > > +int ddlist_free(struct ddlist *list) > > +{ > > + struct ddlist *next; > > + int count = 0; > > + > > + while(list!=NULL){ > > + next = list->next; > > + free(list->device); > > + free(list); > > + list = next; > > + count++; > > + } > > + > > + return count; > > +} > > Let's use glib's linked list routines. The GSList is usable here. > http://library.gnome.org/devel/glib/stable/glib-Singly-Linked-Lists.html > > > +/* > > + * Look for partition with specific label (part of #316481) > > + */ > > +struct ddlist* findDriverDiskByLabel(void) > > +{ > > + char *ddLabel = "OEMDRV"; > > + struct ddlist *ddDevice = NULL; > > + blkid_cache bCache; > > + > > + int res; > > + blkid_dev_iterate bIter; > > + blkid_dev bDev; > > + > > + if(blkid_get_cache(&bCache, NULL)<0){ > > + logMessage(ERROR, _("Cannot initialize cache instance for > blkid")); > > + return NULL; > > + } > > + if((res = blkid_probe_all(bCache))<0){ > > + logMessage(ERROR, _("Cannot probe devices in blkid: %d"), > res); > > + return NULL; > > + } > > + > > + bIter = blkid_dev_iterate_begin(bCache); > > + blkid_dev_set_search(bIter, "LABEL", ddLabel); > > + while((res = blkid_dev_next(bIter, &bDev))!=0){ > > + bDev = blkid_verify(bCache, bDev); > > + if(!bDev) > > + continue; > > + logMessage(DEBUGLVL, _("Adding driver disc %s to the list > of available DDs."), blkid_dev_devname(bDev)); > > + ddDevice = ddlist_add(ddDevice, blkid_dev_devname(bDev)); > > + /*blkid_free_dev(bDev); -- probably taken care of by the > put cache call.. it is not exposed in the API */ > > + } > > + blkid_dev_iterate_end(bIter); > > + > > + blkid_put_cache(bCache); > > + > > + return ddDevice; > > +} > > + > > +int loadDriverDiskFromPartition(struct loaderData_s *loaderData, > char* device) > > +{ > > + int rc; > > + > > + logMessage(INFO, "trying to mount %s", device); > > + if (doPwMount(device, "/tmp/drivers", "auto", "ro", NULL)) { > > + logMessage(ERROR, _("Failed to mount driver disk.")); > > + return -1; > > + } > > + > > + rc = verifyDriverDisk("/tmp/drivers"); > > + if (rc == LOADER_BACK) { > > + logMessage(ERROR, _("Driver disk is invalid for this " > > + "release of %s."), getProductName()); > > + umount("/tmp/drivers"); > > + return -2; > > + } > > + > > + rc = loadDriverDisk(loaderData, "/tmp/drivers"); > > + umount("/tmp/drivers"); > > + if (rc == LOADER_BACK) { > > + return -3; > > + } > > + > > + return 0; > > +} > > + > > diff --git a/loader/driverdisk.h b/loader/driverdisk.h > > index e6e919d..3e442fb 100644 > > --- a/loader/driverdisk.h > > +++ b/loader/driverdisk.h > > @@ -24,6 +24,11 @@ > > #include "modules.h" > > #include "moduleinfo.h" > > > > +#define DD_RPMS "/tmp/DD-%d" > > +#define DD_EXTRACTED "/tmp/DD" > > +#define DD_MODULES "/tmp/DD/lib/modules" > > +#define DD_FIRMWARE "/tmp/DD/lib/firmware" > > + > > extern char *ddFsTypes[]; > > > > int loadDriverFromMedia(int class, struct loaderData_s *loaderData, > > @@ -39,4 +44,19 @@ void useKickstartDD(struct loaderData_s * > loaderData, int argc, > > > > void getDDFromSource(struct loaderData_s * loaderData, char * src); > > > > +int loadDriverDiskFromPartition(struct loaderData_s *loaderData, > char* device); > > + > > +struct ddlist { > > + char* device; > > + struct ddlist* next; > > +}; > > + > > +struct ddlist* ddlist_add(struct ddlist *list, const char* > device); > > +int ddlist_free(struct ddlist *list); > > + > > +struct ddlist* findDriverDiskByLabel(void); > > + > > +int modprobeNormalmode(); > > +int modprobeDDmode(); > > + > > #endif > > diff --git a/loader/loader.c b/loader/loader.c > > index cd3e178..3159513 100644 > > --- a/loader/loader.c > > +++ b/loader/loader.c > > @@ -959,6 +959,10 @@ static void parseCmdLineFlags(struct > loaderData_s * loaderData, > > else if (!strcasecmp(argv[i], "dd") || > > !strcasecmp(argv[i], "driverdisk")) > > flags |= LOADER_FLAGS_MODDISK; > > + else if (!strcasecmp(argv[i], "dlabel=on")) > > + flags |= LOADER_FLAGS_AUTOMODDISK; > > + else if (!strcasecmp(argv[i], "dlabel=off")) > > + flags &= ~LOADER_FLAGS_AUTOMODDISK; > > else if (!strcasecmp(argv[i], "rescue")) > > flags |= LOADER_FLAGS_RESCUE; > > else if (!strcasecmp(argv[i], "nopass")) > > @@ -1785,6 +1789,7 @@ int main(int argc, char ** argv) { > > struct loaderData_s loaderData; > > > > char *path; > > + struct ddlist *dd, *dditer; > > > > gchar *cmdLine = NULL, *ksFile = NULL, *virtpcon = NULL; > > gboolean testing = FALSE, mediacheck = FALSE; > > @@ -1882,6 +1887,12 @@ int main(int argc, char ** argv) { > > flags |= LOADER_FLAGS_NOSHELL; > > #endif > > > > + /* XXX if RHEL, enable the AUTODD feature by default, > > + * but we should come with more general way how to control this > */ > > + if(!strncmp(getProductName(), "Red Hat", 7)){ > > + flags |= LOADER_FLAGS_AUTOMODDISK; > > + } > > + > > openLog(FL_TESTING(flags)); > > if (!FL_TESTING(flags)) > > openlog("loader", 0, LOG_LOCAL0); > > @@ -1946,6 +1957,26 @@ int main(int argc, char ** argv) { > > /* FIXME: this is a bit of a hack */ > > loaderData.modInfo = modInfo; > > > > + /* Setup depmod & modprobe so we can load multiple DDs */ > > + modprobeDDmode(); > > + > > + if(FL_AUTOMODDISK(flags)){ > > + /* Load all autodetected DDs */ > > + logMessage(INFO, "Trying to detect vendor driver discs"); > > + dd = findDriverDiskByLabel(); > > + dditer = dd; > > + while(dditer){ > > + if(loadDriverDiskFromPartition(&loaderData, > dditer->device)){ > > + logMessage(ERROR, "Automatic driver disk loader > failed for %s.", dditer->device); > > + } > > + else{ > > + logMessage(INFO, "Automatic driver disk loader > succeeded for %s.", dditer->device); > > + } > > + dditer = dditer->next; > > + } > > + ddlist_free(dd); > > + } > > + > > if (FL_MODDISK(flags)) { > > startNewt(); > > loadDriverDisks(DEVICE_ANY, &loaderData); > > @@ -1955,6 +1986,9 @@ int main(int argc, char ** argv) { > > logMessage(INFO, "found /dd.img, loading drivers"); > > getDDFromSource(&loaderData, "path:/dd.img"); > > } > > + > > + /* Reset depmod & modprobe to normal mode and get the rest of > drivers*/ > > + modprobeNormalmode(); > > > > /* this allows us to do an early load of modules specified on > the > > * command line to allow automating the load order of modules so > that > > @@ -2139,6 +2173,9 @@ int main(int argc, char ** argv) { > > tmparg++; > > } > > > > + if (FL_AUTOMODDISK(flags)) > > + *argptr++ = "--dlabel"; > > + > > if (FL_NOIPV4(flags)) > > *argptr++ = "--noipv4"; > > > > diff --git a/loader/loader.h b/loader/loader.h > > index ebf3766..ca6404f 100644 > > --- a/loader/loader.h > > +++ b/loader/loader.h > > @@ -70,6 +70,7 @@ > > #define LOADER_FLAGS_HAVE_CMSCONF (((uint64_t) 1) << 37) > > #define LOADER_FLAGS_NOKILL (((uint64_t) 1) << 38) > > #define LOADER_FLAGS_KICKSTART_SEND_SERIAL (((uint64_t) 1) << 39) > > +#define LOADER_FLAGS_AUTOMODDISK (((uint64_t) 1) << 40) > > > > #define FL_TESTING(a) ((a) & LOADER_FLAGS_TESTING) > > #define FL_TEXT(a) ((a) & LOADER_FLAGS_TEXT) > > @@ -107,6 +108,7 @@ > > #define FL_HAVE_CMSCONF(a) ((a) & LOADER_FLAGS_HAVE_CMSCONF) > > #define FL_NOKILL(a) ((a) & LOADER_FLAGS_NOKILL) > > #define FL_KICKSTART_SEND_SERIAL(a) ((a) & > LOADER_FLAGS_KICKSTART_SEND_SERIAL) > > +#define FL_AUTOMODDISK(a) ((a) & LOADER_FLAGS_AUTOMODDISK) > > > > void startNewt(void); > > void stopNewt(void); > > > > Large items: > > - - Fix the coding style. > - - Use glib's linked list rather than creating a new one. > - - Avoid the use of static buffers. This is one thing I like about > the > convenience functions in glib. Almost all will handle dynamic > memory > allocation. I see no reason to not make use of that in loader. > > I've got comments for rpm handling in another email. > > - -- > David Cantrell <dcantrell@xxxxxxxxxx> > Red Hat / Honolulu, HI > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.10 (GNU/Linux) > > iEYEARECAAYFAksdZfsACgkQ5hsjjIy1VkkSmQCePhyqQ0MioUiUz5tmdI1X2TGC > DwgAoKPhf1qCcR+ROTjptAVZnjdGPW9J > =W8vv > -----END PGP SIGNATURE----- > > _______________________________________________ > Anaconda-devel-list mailing list > Anaconda-devel-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/anaconda-devel-list _______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list