Message ID | 1291436933-26861-2-git-send-email-steve@sakoman.com |
---|---|
State | Changes Requested |
Headers | show |
On Friday, December 03, 2010 23:28:51 Steve Sakoman wrote:
> + if (!config_file || !strlen(config_file)) {
strlen is a bad bad idea. if you only want to see if it's non-empty, check
the first byte.
-mike
On 12/04/2010 05:28 AM, Steve Sakoman wrote: > From: Loïc Minier <loic.minier@linaro.org> > Hi, > #if defined(CONFIG_FILE) > + /* Default to the config file specified in FW_CONFIG_FILE */ > + char *config_file = getenv("FW_CONFIG_FILE"); What about to pass the configuration file on the command line instead of an environment variable ? Something like "fw_env -c /etc/fw_config", for example ? IMHO it is easier to understand and we can add documentation in the "help" message. Best regards, Stefano Babic
On Sat, 2010-12-04 at 11:59 +0100, Stefano Babic wrote: > On 12/04/2010 05:28 AM, Steve Sakoman wrote: > > From: Loïc Minier <loic.minier@linaro.org> > > > > Hi, > > > #if defined(CONFIG_FILE) > > + /* Default to the config file specified in FW_CONFIG_FILE */ > > + char *config_file = getenv("FW_CONFIG_FILE"); > > What about to pass the configuration file on the command line instead of > an environment variable ? Something like "fw_env -c /etc/fw_config", for > example ? IMHO it is easier to understand and we can add documentation > in the "help" message. Good suggestion, I prefer that too. Steve
On Sat, 2010-12-04 at 05:34 -0500, Mike Frysinger wrote: > On Friday, December 03, 2010 23:28:51 Steve Sakoman wrote: > > + if (!config_file || !strlen(config_file)) { > > strlen is a bad bad idea. if you only want to see if it's non-empty, check > the first byte. I'll correct this in the next revision. Thanks for reviewing the code! Steve
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 8ff7052..75f6a98 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -1224,8 +1224,15 @@ static int parse_config () struct stat st; #if defined(CONFIG_FILE) + /* Default to the config file specified in FW_CONFIG_FILE */ + char *config_file = getenv("FW_CONFIG_FILE"); + if (!config_file || !strlen(config_file)) { + /* If unset or empty use the default config file */ + config_file = CONFIG_FILE; + } + /* Fills in DEVNAME(), ENVSIZE(), DEVESIZE(). Or don't. */ - if (get_config (CONFIG_FILE)) { + if (get_config (config_file)) { fprintf (stderr, "Cannot parse config file: %s\n", strerror (errno)); return -1;