From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Frysinger Date: Wed, 22 Aug 2012 23:43:36 -0400 Subject: [U-Boot] [PATCH 10/12] env: acl: Add environment variable access control list In-Reply-To: <1345236586-19076-11-git-send-email-joe.hershberger@ni.com> References: <20100622222932.DCDB71524EE@gemini.denx.de> <1345236586-19076-1-git-send-email-joe.hershberger@ni.com> <1345236586-19076-11-git-send-email-joe.hershberger@ni.com> Message-ID: <201208222343.37055.vapier@gentoo.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > +#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@the min/max already defined in include/common.h rather than duplicating another one locally. -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. URL: