From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:60348) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T2OvS-0001c6-KQ for qemu-devel@nongnu.org; Fri, 17 Aug 2012 11:57:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T2OvQ-0004nf-Su for qemu-devel@nongnu.org; Fri, 17 Aug 2012 11:57:30 -0400 Received: from cantor2.suse.de ([195.135.220.15]:42287 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T2OvQ-0004nX-F9 for qemu-devel@nongnu.org; Fri, 17 Aug 2012 11:57:28 -0400 Message-ID: <502E69E4.4050306@suse.de> Date: Fri, 17 Aug 2012 17:57:24 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1337681798-22395-1-git-send-email-jim@meyering.net> <1337681798-22395-2-git-send-email-jim@meyering.net> In-Reply-To: <1337681798-22395-2-git-send-email-jim@meyering.net> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCHv3 1/2] envlist.c: convert each TAB(width-4) to equivalent spaces List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jim Meyering Cc: Jim Meyering , qemu-devel@nongnu.org Am 22.05.2012 12:16, schrieb Jim Meyering: > From: Jim Meyering >=20 >=20 > Signed-off-by: Jim Meyering > --- > envlist.c | 256 +++++++++++++++++++++++++++++++-----------------------= -------- > 1 file changed, 128 insertions(+), 128 deletions(-) >=20 > diff --git a/envlist.c b/envlist.c > index f2303cd..e44889b 100644 > --- a/envlist.c > +++ b/envlist.c > @@ -8,13 +8,13 @@ > #include "envlist.h" >=20 > struct envlist_entry { > - const char *ev_var; /* actual env value */ > - QLIST_ENTRY(envlist_entry) ev_link; > + const char *ev_var; /* actual env value */ > + QLIST_ENTRY(envlist_entry) ev_link; > }; >=20 > struct envlist { > - QLIST_HEAD(, envlist_entry) el_entries; /* actual entries */ > - size_t el_count; /* number of entries */ > + QLIST_HEAD(, envlist_entry) el_entries; /* actual entries */ > + size_t el_count; /* number of entries */ > }; >=20 > static int envlist_parse(envlist_t *envlist, > @@ -27,15 +27,15 @@ static int envlist_parse(envlist_t *envlist, > envlist_t * > envlist_create(void) > { > - envlist_t *envlist; > + envlist_t *envlist; >=20 > - if ((envlist =3D malloc(sizeof (*envlist))) =3D=3D NULL) > - return (NULL); > + if ((envlist =3D malloc(sizeof (*envlist))) =3D=3D NULL) > + return (NULL); Please add braces. >=20 > - QLIST_INIT(&envlist->el_entries); > - envlist->el_count =3D 0; > + QLIST_INIT(&envlist->el_entries); > + envlist->el_count =3D 0; >=20 > - return (envlist); > + return (envlist); > } >=20 > /* > @@ -44,18 +44,18 @@ envlist_create(void) > void > envlist_free(envlist_t *envlist) > { > - struct envlist_entry *entry; > + struct envlist_entry *entry; >=20 > - assert(envlist !=3D NULL); > + assert(envlist !=3D NULL); >=20 > - while (envlist->el_entries.lh_first !=3D NULL) { > - entry =3D envlist->el_entries.lh_first; > - QLIST_REMOVE(entry, ev_link); > + while (envlist->el_entries.lh_first !=3D NULL) { > + entry =3D envlist->el_entries.lh_first; > + QLIST_REMOVE(entry, ev_link); >=20 > - free((char *)entry->ev_var); > - free(entry); > - } > - free(envlist); > + free((char *)entry->ev_var); > + free(entry); > + } > + free(envlist); > } >=20 > /* > @@ -72,7 +72,7 @@ envlist_free(envlist_t *envlist) > int > envlist_parse_set(envlist_t *envlist, const char *env) > { > - return (envlist_parse(envlist, env, &envlist_setenv)); > + return (envlist_parse(envlist, env, &envlist_setenv)); > } >=20 > /* > @@ -84,7 +84,7 @@ envlist_parse_set(envlist_t *envlist, const char *env= ) > int > envlist_parse_unset(envlist_t *envlist, const char *env) > { > - return (envlist_parse(envlist, env, &envlist_unsetenv)); > + return (envlist_parse(envlist, env, &envlist_unsetenv)); > } >=20 > /* > @@ -97,32 +97,32 @@ static int > envlist_parse(envlist_t *envlist, const char *env, > int (*callback)(envlist_t *, const char *)) > { > - char *tmpenv, *envvar; > - char *envsave =3D NULL; > - > - assert(callback !=3D NULL); > - > - if ((envlist =3D=3D NULL) || (env =3D=3D NULL)) > - return (EINVAL); > - > - /* > - * We need to make temporary copy of the env string > - * as strtok_r(3) modifies it while it tokenizes. > - */ > - if ((tmpenv =3D strdup(env)) =3D=3D NULL) > - return (errno); > - > - envvar =3D strtok_r(tmpenv, ",", &envsave); > - while (envvar !=3D NULL) { > - if ((*callback)(envlist, envvar) !=3D 0) { > - free(tmpenv); > - return (errno); > - } > - envvar =3D strtok_r(NULL, ",", &envsave); > - } > - > - free(tmpenv); > - return (0); > + char *tmpenv, *envvar; > + char *envsave =3D NULL; > + > + assert(callback !=3D NULL); > + > + if ((envlist =3D=3D NULL) || (env =3D=3D NULL)) > + return (EINVAL); Braces > + > + /* > + * We need to make temporary copy of the env string > + * as strtok_r(3) modifies it while it tokenizes. > + */ > + if ((tmpenv =3D strdup(env)) =3D=3D NULL) > + return (errno); Braces > + > + envvar =3D strtok_r(tmpenv, ",", &envsave); > + while (envvar !=3D NULL) { > + if ((*callback)(envlist, envvar) !=3D 0) { > + free(tmpenv); > + return (errno); > + } > + envvar =3D strtok_r(NULL, ",", &envsave); > + } > + > + free(tmpenv); > + return (0); > } >=20 > /* > @@ -134,46 +134,46 @@ envlist_parse(envlist_t *envlist, const char *env= , > int > envlist_setenv(envlist_t *envlist, const char *env) > { > - struct envlist_entry *entry =3D NULL; > - const char *eq_sign; > - size_t envname_len; > - > - if ((envlist =3D=3D NULL) || (env =3D=3D NULL)) > - return (EINVAL); > - > - /* find out first equals sign in given env */ > - if ((eq_sign =3D strchr(env, '=3D')) =3D=3D NULL) > - return (EINVAL); > - envname_len =3D eq_sign - env + 1; > - > - /* > - * If there already exists variable with given name > - * we remove and release it before allocating a whole > - * new entry. > - */ > - for (entry =3D envlist->el_entries.lh_first; entry !=3D NULL; > - entry =3D entry->ev_link.le_next) { > - if (strncmp(entry->ev_var, env, envname_len) =3D=3D 0) > - break; > - } > - > - if (entry !=3D NULL) { > - QLIST_REMOVE(entry, ev_link); > - free((char *)entry->ev_var); > - free(entry); > - } else { > - envlist->el_count++; > - } > - > - if ((entry =3D malloc(sizeof (*entry))) =3D=3D NULL) > - return (errno); > - if ((entry->ev_var =3D strdup(env)) =3D=3D NULL) { > - free(entry); > - return (errno); > - } > - QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link); > - > - return (0); > + struct envlist_entry *entry =3D NULL; > + const char *eq_sign; > + size_t envname_len; > + > + if ((envlist =3D=3D NULL) || (env =3D=3D NULL)) > + return (EINVAL); Braces > + > + /* find out first equals sign in given env */ > + if ((eq_sign =3D strchr(env, '=3D')) =3D=3D NULL) > + return (EINVAL); Braces > + envname_len =3D eq_sign - env + 1; > + > + /* > + * If there already exists variable with given name > + * we remove and release it before allocating a whole > + * new entry. > + */ > + for (entry =3D envlist->el_entries.lh_first; entry !=3D NULL; > + entry =3D entry->ev_link.le_next) { This adds an off-by-one that I believe you fix in 2/2. > + if (strncmp(entry->ev_var, env, envname_len) =3D=3D 0) > + break; Braces > + } > + > + if (entry !=3D NULL) { > + QLIST_REMOVE(entry, ev_link); > + free((char *)entry->ev_var); > + free(entry); > + } else { > + envlist->el_count++; > + } > + > + if ((entry =3D malloc(sizeof (*entry))) =3D=3D NULL) > + return (errno); Braces > + if ((entry->ev_var =3D strdup(env)) =3D=3D NULL) { > + free(entry); > + return (errno); > + } > + QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link); > + > + return (0); > } >=20 > /* > @@ -183,34 +183,34 @@ envlist_setenv(envlist_t *envlist, const char *en= v) > int > envlist_unsetenv(envlist_t *envlist, const char *env) > { > - struct envlist_entry *entry; > - size_t envname_len; > - > - if ((envlist =3D=3D NULL) || (env =3D=3D NULL)) > - return (EINVAL); > - > - /* env is not allowed to contain '=3D' */ > - if (strchr(env, '=3D') !=3D NULL) > - return (EINVAL); > - > - /* > - * Find out the requested entry and remove > - * it from the list. > - */ > - envname_len =3D strlen(env); > - for (entry =3D envlist->el_entries.lh_first; entry !=3D NULL; > - entry =3D entry->ev_link.le_next) { > - if (strncmp(entry->ev_var, env, envname_len) =3D=3D 0) > - break; > - } > - if (entry !=3D NULL) { > - QLIST_REMOVE(entry, ev_link); > - free((char *)entry->ev_var); > - free(entry); > - > - envlist->el_count--; > - } > - return (0); > + struct envlist_entry *entry; > + size_t envname_len; > + > + if ((envlist =3D=3D NULL) || (env =3D=3D NULL)) > + return (EINVAL); Braces > + > + /* env is not allowed to contain '=3D' */ > + if (strchr(env, '=3D') !=3D NULL) > + return (EINVAL); Braces > + > + /* > + * Find out the requested entry and remove > + * it from the list. > + */ > + envname_len =3D strlen(env); > + for (entry =3D envlist->el_entries.lh_first; entry !=3D NULL; > + entry =3D entry->ev_link.le_next) { Or was it this off-by-one? > + if (strncmp(entry->ev_var, env, envname_len) =3D=3D 0) > + break; Braces > + } > + if (entry !=3D NULL) { > + QLIST_REMOVE(entry, ev_link); > + free((char *)entry->ev_var); > + free(entry); > + > + envlist->el_count--; > + } > + return (0); > } >=20 > /* > @@ -226,21 +226,21 @@ envlist_unsetenv(envlist_t *envlist, const char *= env) > char ** > envlist_to_environ(const envlist_t *envlist, size_t *count) > { > - struct envlist_entry *entry; > - char **env, **penv; > + struct envlist_entry *entry; > + char **env, **penv; >=20 > - penv =3D env =3D malloc((envlist->el_count + 1) * sizeof (char *)); > - if (env =3D=3D NULL) > - return (NULL); > + penv =3D env =3D malloc((envlist->el_count + 1) * sizeof (char *))= ; > + if (env =3D=3D NULL) > + return (NULL); Braces >=20 > - for (entry =3D envlist->el_entries.lh_first; entry !=3D NULL; > - entry =3D entry->ev_link.le_next) { > - *(penv++) =3D strdup(entry->ev_var); > - } > - *penv =3D NULL; /* NULL terminate the list */ > + for (entry =3D envlist->el_entries.lh_first; entry !=3D NULL; > + entry =3D entry->ev_link.le_next) { align? > + *(penv++) =3D strdup(entry->ev_var); > + } > + *penv =3D NULL; /* NULL terminate the list */ >=20 > - if (count !=3D NULL) > - *count =3D envlist->el_count; > + if (count !=3D NULL) > + *count =3D envlist->el_count; Braces >=20 > - return (env); > + return (env); > } >=20 I've commented on all Coding Style issues I've spotted - basically, the convention has been to make the patch please checkpatch.pl and CODING_STYLE on all added lines. Didn't notice any particular deviation or mismerge so this looks okay for 1.2 if you can send a v4. Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg