On Friday 17 August 2012 16:49:44 Joe Hershberger wrote: > --- a/common/cmd_nvedit.c > +++ b/common/cmd_nvedit.c > > +#if defined(CONFIG_ENV_ACL) > +#include <env_acl.h> > +#endif
the header should not need protection just to be included
> +#ifdef CONFIG_ENV_ACL
> + if (env_acl_validate_env_set_params(argc, argv) < 0)
> + return 1;
> +#endif
have the header define env_acl_validate_env_set_params() as a return 0 static
inline func when CONFIG_ENV_ACL isn't defined and then you can drop the ifdef
here
> --- /dev/null
> +++ b/common/env_acl.c
>
> + * (C) Copyright 2010
fwiw, it's 2012 now
> +static int _env_acl_lookup_r(const char *name, char *attributes, int
> static_acl)
> ...
> + entry = strstr(acl, name);
> + while (entry != NULL) {
> + if ((entry == acl || *(entry - 1) == ENV_ACL_LIST_DELIM ||
> + *(entry - 1) == ' ') &&
> + (*(entry + strlen(name)) == ENV_ACL_ATTR_SEP ||
> + *(entry + strlen(name)) == ENV_ACL_LIST_DELIM ||
> + *(entry + strlen(name)) == '\0' ||
> + *(entry + strlen(name)) == ' '))
> + break;
is that strlen optimized away ? i suspect not. and even if it is, the
duplication here is kind of ugly, so it'd be better to use a local var
anyways.
const char *acl_val = entry + strlen(name);
> +static int env_acl_lookup_r(const char *name, char *attributes)
> +{
> + int ret_val;
> + /* try the env first */
> + ret_val = _env_acl_lookup_r(name, attributes, 0);
> + if (ret_val != 0) {
> + /* if not found in the env, look in the static list */
> + ret_val = _env_acl_lookup_r(name, attributes, 1);
> + }
> + return ret_val;
> +}
> +
> +enum env_acl_var_type env_acl_get_type(const char *name)
> +{
> + char *type;
const
> +static void skip_num(int hex, const char *value, const char **end,
> + int max_digits)
> +{
> + int i;
> +
> + if (hex && is_hex_prefix(value))
> + value += 2;
> +
> + for (i = max_digits; i != 0; i--) {
> + if (hex && !isxdigit(*value))
> + break;
> + if (!hex && !isdigit(*value))
> + break;
> + value++;
> + }
> + if (end != NULL)
> + *end = value;
> +}
couldn't you use strtol and abuse the endptr field ?
> --- a/tools/env/fw_env.h
> +++ b/tools/env/fw_env.h
>
> +#define min(x, y) ({ \
> + typeof(x) _min1 = (x); \
> + typeof(y) _min2 = (y); \
> + (void) (&_min1 == &_min2); \
> + _min1 < _min2 ? _min1 : _min2; })
ugh, no. use include/compiler.h. you might want to look at the min/max
already defined in include/common.h rather than duplicating another one
locally.
-mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
