Hi Khem,
* Khem Raj <raj.khem@xxxxxxxxx> [2012-02-02 23:09:59 -0800]:
> fopen() will not reset errno if it succeeds so we should
> make sure that we only use errno in error cases.
>
> Also fix the diagnostic messages to not use strerror
> when there is no error since strerror will not return
> anything useful in this case
>
> Signed-off-by: Khem Raj <raj.khem@xxxxxxxxx>
> ---
> tools/kmod-depmod.c | 29 ++++++++++++++++-------------
> 1 files changed, 16 insertions(+), 13 deletions(-)
Patch looks good, with only some nits... 1st one is that we don't use
s-o-b.
>
> diff --git a/tools/kmod-depmod.c b/tools/kmod-depmod.c
> index 0721daf..01bca2a 100644
> --- a/tools/kmod-depmod.c
> +++ b/tools/kmod-depmod.c
> @@ -941,7 +941,7 @@ static int cfg_files_list(struct cfg_file ***p_files, size_t *p_n_files,
> }
>
> closedir(d);
> - DBG("parsed configuration files from %s: %s\n", path, strerror(-err));
> + DBG("parsed configuration files from %s\n", path);
> return err;
> }
>
> @@ -2293,13 +2293,15 @@ static int depmod_load_symvers(struct depmod *depmod, const char *filename)
> char line[10240];
> FILE *fp;
> unsigned int linenum = 0;
> - int err;
> + int err = 0;
>
> fp = fopen(filename, "r");
> - err = -errno;
> - DBG("load symvers: %s: %s\n", filename, strerror(-err));
> - if (fp == NULL)
> + if (fp == NULL) {
> + err = -errno;
> + DBG("load symvers: %s: %s\n", filename, strerror(-err));
> return err;
> + }
> + DBG("load symvers: %s\n", filename);
Since we are using err only in the error path fp==NULL, it's better to
completely remove it here. Are you ok with the amended patch below?
Lucas De Marchi
>From bb4cf563ebbec0d8e13eed22bd2613da72c991d1 Mon Sep 17 00:00:00 2001
From: Khem Raj <raj.khem@xxxxxxxxx>
Date: Thu, 2 Feb 2012 23:09:59 -0800
Subject: [PATCH] depmod: Dont use errno unconditionally
fopen() will not reset errno if it succeeds so we should
make sure that we only use errno in error cases.
Also fix the diagnostic messages to not use strerror
when there is no error since strerror will not return
anything useful in this case
---
tools/kmod-depmod.c | 30 ++++++++++++++++--------------
1 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/tools/kmod-depmod.c b/tools/kmod-depmod.c
index 0721daf..1871e18 100644
--- a/tools/kmod-depmod.c
+++ b/tools/kmod-depmod.c
@@ -941,7 +941,7 @@ static int cfg_files_list(struct cfg_file ***p_files, size_t *p_n_files,
}
closedir(d);
- DBG("parsed configuration files from %s: %s\n", path, strerror(-err));
+ DBG("parsed configuration files from %s\n", path);
return err;
}
@@ -2293,13 +2293,14 @@ static int depmod_load_symvers(struct depmod *depmod, const char *filename)
char line[10240];
FILE *fp;
unsigned int linenum = 0;
- int err;
fp = fopen(filename, "r");
- err = -errno;
- DBG("load symvers: %s: %s\n", filename, strerror(-err));
- if (fp == NULL)
+ if (fp == NULL) {
+ int err = -errno;
+ DBG("load symvers: %s: %m\n", filename);
return err;
+ }
+ DBG("load symvers: %s\n", filename);
/* eg. "0xb352177e\tfind_first_bit\tvmlinux\tEXPORT_SYMBOL" */
while (fgets(line, sizeof(line), fp) != NULL) {
@@ -2329,10 +2330,10 @@ static int depmod_load_symvers(struct depmod *depmod, const char *filename)
}
depmod_add_fake_syms(depmod);
- DBG("loaded symvers: %s: %s\n", filename, strerror(-err));
+ DBG("loaded symvers: %s\n", filename);
fclose(fp);
- return err;
+ return 0;
}
static int depmod_load_system_map(struct depmod *depmod, const char *filename)
@@ -2342,13 +2343,14 @@ static int depmod_load_system_map(struct depmod *depmod, const char *filename)
char line[10240];
FILE *fp;
unsigned int linenum = 0;
- int err;
fp = fopen(filename, "r");
- err = -errno;
- DBG("load System.map: %s: %s\n", filename, strerror(-err));
- if (fp == NULL)
+ if (fp == NULL) {
+ int err = -errno;
+ DBG("load System.map: %s: %m\n", filename);
return err;
+ }
+ DBG("load System.map: %s\n", filename);
/* eg. c0294200 R __ksymtab_devfs_alloc_devnum */
while (fgets(line, sizeof(line), fp) != NULL) {
@@ -2381,10 +2383,10 @@ static int depmod_load_system_map(struct depmod *depmod, const char *filename)
}
depmod_add_fake_syms(depmod);
- DBG("loaded System.map: %s: %s\n", filename, strerror(-err));
+ DBG("loaded System.map: %s\n", filename);
fclose(fp);
- return err;
+ return 0;
}
@@ -2673,7 +2675,7 @@ static int do_depmod(int argc, char *argv[])
} else if (system_map != NULL) {
err = depmod_load_system_map(&depmod, system_map);
if (err < 0) {
- CRIT("could not load %s: %s\n", module_symvers,
+ CRIT("could not load %s: %s\n", system_map,
strerror(-err));
goto cmdline_failed;
}
--
1.7.9
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
