From: "Andreas Färber" <afaerber@suse.de>
To: Jim Meyering <jim@meyering.net>
Cc: Jim Meyering <meyering@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCHv3 1/2] envlist.c: convert each TAB(width-4) to equivalent spaces
Date: Fri, 17 Aug 2012 17:57:24 +0200 [thread overview]
Message-ID: <502E69E4.4050306@suse.de> (raw)
In-Reply-To: <1337681798-22395-2-git-send-email-jim@meyering.net>
Am 22.05.2012 12:16, schrieb Jim Meyering:
> From: Jim Meyering <meyering@redhat.com>
>
>
> Signed-off-by: Jim Meyering <meyering@redhat.com>
> ---
> envlist.c | 256 +++++++++++++++++++++++++++++++-------------------------------
> 1 file changed, 128 insertions(+), 128 deletions(-)
>
> 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"
>
> 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;
> };
>
> 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 */
> };
>
> 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;
>
> - if ((envlist = malloc(sizeof (*envlist))) == NULL)
> - return (NULL);
> + if ((envlist = malloc(sizeof (*envlist))) == NULL)
> + return (NULL);
Please add braces.
>
> - QLIST_INIT(&envlist->el_entries);
> - envlist->el_count = 0;
> + QLIST_INIT(&envlist->el_entries);
> + envlist->el_count = 0;
>
> - return (envlist);
> + return (envlist);
> }
>
> /*
> @@ -44,18 +44,18 @@ envlist_create(void)
> void
> envlist_free(envlist_t *envlist)
> {
> - struct envlist_entry *entry;
> + struct envlist_entry *entry;
>
> - assert(envlist != NULL);
> + assert(envlist != NULL);
>
> - while (envlist->el_entries.lh_first != NULL) {
> - entry = envlist->el_entries.lh_first;
> - QLIST_REMOVE(entry, ev_link);
> + while (envlist->el_entries.lh_first != NULL) {
> + entry = envlist->el_entries.lh_first;
> + QLIST_REMOVE(entry, ev_link);
>
> - free((char *)entry->ev_var);
> - free(entry);
> - }
> - free(envlist);
> + free((char *)entry->ev_var);
> + free(entry);
> + }
> + free(envlist);
> }
>
> /*
> @@ -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));
> }
>
> /*
> @@ -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));
> }
>
> /*
> @@ -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 = NULL;
> -
> - assert(callback != NULL);
> -
> - if ((envlist == NULL) || (env == NULL))
> - return (EINVAL);
> -
> - /*
> - * We need to make temporary copy of the env string
> - * as strtok_r(3) modifies it while it tokenizes.
> - */
> - if ((tmpenv = strdup(env)) == NULL)
> - return (errno);
> -
> - envvar = strtok_r(tmpenv, ",", &envsave);
> - while (envvar != NULL) {
> - if ((*callback)(envlist, envvar) != 0) {
> - free(tmpenv);
> - return (errno);
> - }
> - envvar = strtok_r(NULL, ",", &envsave);
> - }
> -
> - free(tmpenv);
> - return (0);
> + char *tmpenv, *envvar;
> + char *envsave = NULL;
> +
> + assert(callback != NULL);
> +
> + if ((envlist == NULL) || (env == 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 = strdup(env)) == NULL)
> + return (errno);
Braces
> +
> + envvar = strtok_r(tmpenv, ",", &envsave);
> + while (envvar != NULL) {
> + if ((*callback)(envlist, envvar) != 0) {
> + free(tmpenv);
> + return (errno);
> + }
> + envvar = strtok_r(NULL, ",", &envsave);
> + }
> +
> + free(tmpenv);
> + return (0);
> }
>
> /*
> @@ -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 = NULL;
> - const char *eq_sign;
> - size_t envname_len;
> -
> - if ((envlist == NULL) || (env == NULL))
> - return (EINVAL);
> -
> - /* find out first equals sign in given env */
> - if ((eq_sign = strchr(env, '=')) == NULL)
> - return (EINVAL);
> - envname_len = 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 = envlist->el_entries.lh_first; entry != NULL;
> - entry = entry->ev_link.le_next) {
> - if (strncmp(entry->ev_var, env, envname_len) == 0)
> - break;
> - }
> -
> - if (entry != NULL) {
> - QLIST_REMOVE(entry, ev_link);
> - free((char *)entry->ev_var);
> - free(entry);
> - } else {
> - envlist->el_count++;
> - }
> -
> - if ((entry = malloc(sizeof (*entry))) == NULL)
> - return (errno);
> - if ((entry->ev_var = strdup(env)) == NULL) {
> - free(entry);
> - return (errno);
> - }
> - QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
> -
> - return (0);
> + struct envlist_entry *entry = NULL;
> + const char *eq_sign;
> + size_t envname_len;
> +
> + if ((envlist == NULL) || (env == NULL))
> + return (EINVAL);
Braces
> +
> + /* find out first equals sign in given env */
> + if ((eq_sign = strchr(env, '=')) == NULL)
> + return (EINVAL);
Braces
> + envname_len = 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 = envlist->el_entries.lh_first; entry != NULL;
> + entry = 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) == 0)
> + break;
Braces
> + }
> +
> + if (entry != NULL) {
> + QLIST_REMOVE(entry, ev_link);
> + free((char *)entry->ev_var);
> + free(entry);
> + } else {
> + envlist->el_count++;
> + }
> +
> + if ((entry = malloc(sizeof (*entry))) == NULL)
> + return (errno);
Braces
> + if ((entry->ev_var = strdup(env)) == NULL) {
> + free(entry);
> + return (errno);
> + }
> + QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
> +
> + return (0);
> }
>
> /*
> @@ -183,34 +183,34 @@ envlist_setenv(envlist_t *envlist, const char *env)
> int
> envlist_unsetenv(envlist_t *envlist, const char *env)
> {
> - struct envlist_entry *entry;
> - size_t envname_len;
> -
> - if ((envlist == NULL) || (env == NULL))
> - return (EINVAL);
> -
> - /* env is not allowed to contain '=' */
> - if (strchr(env, '=') != NULL)
> - return (EINVAL);
> -
> - /*
> - * Find out the requested entry and remove
> - * it from the list.
> - */
> - envname_len = strlen(env);
> - for (entry = envlist->el_entries.lh_first; entry != NULL;
> - entry = entry->ev_link.le_next) {
> - if (strncmp(entry->ev_var, env, envname_len) == 0)
> - break;
> - }
> - if (entry != 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 == NULL) || (env == NULL))
> + return (EINVAL);
Braces
> +
> + /* env is not allowed to contain '=' */
> + if (strchr(env, '=') != NULL)
> + return (EINVAL);
Braces
> +
> + /*
> + * Find out the requested entry and remove
> + * it from the list.
> + */
> + envname_len = strlen(env);
> + for (entry = envlist->el_entries.lh_first; entry != NULL;
> + entry = entry->ev_link.le_next) {
Or was it this off-by-one?
> + if (strncmp(entry->ev_var, env, envname_len) == 0)
> + break;
Braces
> + }
> + if (entry != NULL) {
> + QLIST_REMOVE(entry, ev_link);
> + free((char *)entry->ev_var);
> + free(entry);
> +
> + envlist->el_count--;
> + }
> + return (0);
> }
>
> /*
> @@ -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;
>
> - penv = env = malloc((envlist->el_count + 1) * sizeof (char *));
> - if (env == NULL)
> - return (NULL);
> + penv = env = malloc((envlist->el_count + 1) * sizeof (char *));
> + if (env == NULL)
> + return (NULL);
Braces
>
> - for (entry = envlist->el_entries.lh_first; entry != NULL;
> - entry = entry->ev_link.le_next) {
> - *(penv++) = strdup(entry->ev_var);
> - }
> - *penv = NULL; /* NULL terminate the list */
> + for (entry = envlist->el_entries.lh_first; entry != NULL;
> + entry = entry->ev_link.le_next) {
align?
> + *(penv++) = strdup(entry->ev_var);
> + }
> + *penv = NULL; /* NULL terminate the list */
>
> - if (count != NULL)
> - *count = envlist->el_count;
> + if (count != NULL)
> + *count = envlist->el_count;
Braces
>
> - return (env);
> + return (env);
> }
>
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
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
next prev parent reply other threads:[~2012-08-17 15:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-22 10:16 [Qemu-devel] [PATCHv3 0/2] envlist.c: handle strdup failure Jim Meyering
2012-05-22 10:16 ` [Qemu-devel] [PATCHv3 1/2] envlist.c: convert each TAB(width-4) to equivalent spaces Jim Meyering
2012-08-17 15:57 ` Andreas Färber [this message]
2012-05-22 10:16 ` [Qemu-devel] [PATCHv3 2/2] envlist.c: handle strdup failure Jim Meyering
2012-08-17 16:03 ` Andreas Färber
2012-08-17 18:30 ` Jim Meyering
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=502E69E4.4050306@suse.de \
--to=afaerber@suse.de \
--cc=jim@meyering.net \
--cc=meyering@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.