From owner-freebsd-bugs Sun Mar 10 9:10:15 2002 Delivered-To: freebsd-bugs@hub.freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id B76E737B417 for ; Sun, 10 Mar 2002 09:10:02 -0800 (PST) Received: (from gnats@localhost) by freefall.freebsd.org (8.11.6/8.11.6) id g2AHA2412633; Sun, 10 Mar 2002 09:10:02 -0800 (PST) (envelope-from gnats) Date: Sun, 10 Mar 2002 09:10:02 -0800 (PST) Message-Id: <200203101710.g2AHA2412633@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org Cc: From: David Malone Subject: Re: kern/35712: kern_linker.c rev. 1.75 and newer break module loading Reply-To: David Malone Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org The following reply was made to PR kern/35712; it has been noted by GNATS. From: David Malone To: "Steven G. Kargl" Cc: FreeBSD-gnats-submit@freebsd.org, ashp@freebsd.org Subject: Re: kern/35712: kern_linker.c rev. 1.75 and newer break module loading Date: Sun, 10 Mar 2002 17:03:06 +0000 On Sat, Mar 09, 2002 at 12:57:47PM -0800, Steven G. Kargl wrote: > As the commit message for revision 1.75 states "Massive > style fixup", I suspect a typo slipped in. However, > this is a +1200 line diff and I haven't been able to > find it. I checked out both copies of the file and had a look with diff -w -u, which helped a bit. There were some odd style changes in this file. In some places, if (!strcmp(lf->filename, koname)) was replaced with: err = strcmp(lf->filename, koname); if (err == 0) which is weird, as strcmp doesn't return an error. The more idiomatic: if (strcmp(lf->filename, koname) == 0) would probably have been better. In fact, the bug you are seeing is due to one of these changes. It is clobbering the value of error, which the surounding code expects to remain unchanged. I've included a patch below which converts these strcmp calls to something more sensible and fixes the bug in the process. Some comments were moved above if statements to facilitate the removal of braces and now the comments don't quite make as much sense now, but that's probably less serious. I'll commit the patch below in a few days, unless Ashley has any problems with it. David. Index: kern_linker.c =================================================================== RCS file: /cvs/FreeBSD-CVS/src/sys/kern/kern_linker.c,v retrieving revision 1.77 diff -u -r1.77 kern_linker.c --- kern_linker.c 27 Feb 2002 18:32:12 -0000 1.77 +++ kern_linker.c 10 Mar 2002 16:54:51 -0000 @@ -362,7 +362,6 @@ { linker_file_t lf = 0; char *koname; - int err; koname = malloc(strlen(filename) + 4, M_LINKER, M_WAITOK); if (koname == NULL) @@ -371,11 +370,9 @@ lockmgr(&lock, LK_SHARED, 0, curthread); TAILQ_FOREACH(lf, &linker_files, link) { - err = strcmp(lf->filename, koname); - if (err == 0) + if (strcmp(lf->filename, koname) == 0) break; - err = strcmp(lf->filename, filename); - if (err == 0) + if (strcmp(lf->filename, filename) == 0) break; } lockmgr(&lock, LK_RELEASE, 0, curthread); @@ -549,7 +546,7 @@ linker_symval_t symval; caddr_t address; size_t common_size = 0; - int err, i; + int i; KLD_DPF(SYM, ("linker_file_lookup_symbol: file=%x, name=%s, deps=%d\n", file, name, deps)); @@ -589,8 +586,7 @@ struct common_symbol *cp; STAILQ_FOREACH(cp, &file->common, link) { - err = strcmp(cp->name, name); - if (err == 0) { + if (strcmp(cp->name, name) == 0) { KLD_DPF(SYM, ("linker_file_lookup_symbol:" " old common value=%x\n", cp->address)); return (cp->address); @@ -983,11 +979,10 @@ modlist_lookup(const char *name, int ver) { modlist_t mod; - int err; TAILQ_FOREACH(mod, &found_modules, link) { - err = strcmp(mod->name, name); - if (err == 0 && (ver == 0 || mod->version == ver)) + if (strcmp(mod->name, name) == 0 && + (ver == 0 || mod->version == ver)) return (mod); } return (NULL); @@ -997,15 +992,14 @@ modlist_lookup2(const char *name, struct mod_depend *verinfo) { modlist_t mod, bestmod; - int err, ver; + int ver; if (verinfo == NULL) return (modlist_lookup(name, 0)); bestmod = NULL; for (mod = TAILQ_FIRST(&found_modules); mod; mod = TAILQ_NEXT(mod, link)) { - err = strcmp(mod->name, name); - if (err != 0) + if (strcmp(mod->name, name) != 0) continue; ver = mod->version; if (ver == verinfo->md_ver_preferred) @@ -1189,8 +1183,7 @@ NULL); nmodname = linker_reloc_ptr(lf, nmp->md_cval); - error = strcmp(modname, nmodname); - if (error == 0) + if (strcmp(modname, nmodname) == 0) break; } if (nmdp < stop) /* it's a self reference */ @@ -1714,8 +1707,7 @@ if (nmp->md_type != MDT_VERSION) continue; nmodname = linker_reloc_ptr(lf, nmp->md_cval); - error = strcmp(modname, nmodname); - if (error == 0) + if (strcmp(modname, nmodname) == 0) break; } if (nmdp < stop)/* early exit, it's a self reference */ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message