| Message ID | 1345803102-21110-8-git-send-email-gerlando.falauto@keymile.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
Dear Gerlando Falauto, > When variables explicitly specified on the command line are not present > in the imported env, delete them from the running env. > If the variable is also missing from the running env, issue a warning. > > Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com> Whew! I made it through ... it wasn't that scary in the end ;-) > --- > lib/hashtable.c | 48 +++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 41 insertions(+), 7 deletions(-) > > diff --git a/lib/hashtable.c b/lib/hashtable.c > index f3f47de..b3d0b64 100644 > --- a/lib/hashtable.c > +++ b/lib/hashtable.c > @@ -607,22 +607,32 @@ ssize_t hexport_r(struct hsearch_data *htab, const > char sep, * himport() > */ > > -/* Check whether variable name is amongst vars[] */ > -static int is_var_in_set(const char *name, int nvars, char * const vars[]) > +/* > + * Check whether variable 'name' is amongst vars[], > + * and remove all instances by setting the pointer to NULL > + */ > +static int is_var_in_set(const char *name, int nvars, char * vars[]) > { > int i = 0; > + int res = 0; > > /* No variables specified means process all of them */ > if (nvars == 0) > return 1; > > for (i = 0; i < nvars; i++) { > - if (!strcmp(name, vars[i])) > - return 1; > + if (vars[i] == NULL) > + continue; > + /* If we found it, delete all of them */ > + if (!strcmp(name, vars[i])) { > + vars[i] = NULL; > + res = 1; break here ? > + } > } > - debug("Skipping non-listed variable %s\n", name); > + if (!res) > + debug("Skipping non-listed variable %s\n", name); > > - return 0; > + return res; > } > > /* > @@ -662,9 +672,11 @@ static int is_var_in_set(const char *name, int nvars, > char * const vars[]) > > int himport_r(struct hsearch_data *htab, > const char *env, size_t size, const char sep, int flag, > - int nvars, char * const vars[], int do_apply) > + int nvars, char * const __vars[], int do_apply) Two underscores are reserved, use something else ;-) > { > char *data, *sp, *dp, *name, *value; > + char *vars[nvars]; > + int i; > > /* Test for correct arguments. */ > if (htab == NULL) { > @@ -681,6 +693,10 @@ int himport_r(struct hsearch_data *htab, > memcpy(data, env, size); > dp = data; > > + /* make a local copy of the list of variables */ > + if (nvars) > + memcpy(vars, __vars, sizeof(__vars[0]) * nvars); > + > if ((flag & H_NOCLEAR) == 0) { > /* Destroy old hash table if one exists */ > debug("Destroy Hash Table: %p table = %p\n", htab, > @@ -809,6 +825,24 @@ int himport_r(struct hsearch_data *htab, > debug("INSERT: free(data = %p)\n", data); > free(data); > > + /* process variables which were not considered */ > + for (i = 0; i < nvars; i++) { > + if (vars[i] == NULL) > + continue; > + /* > + * All variables which were not deleted from the variable list > + * were not present in the imported env > + * This could mean two things: > + * a) if the variable was present in current env, we delete it > + * b) if the variable was not present in current env, we notify > + * it might be a typo > + */ > + if (hdelete_r(vars[i], htab, do_apply) == 0) > + printf("WARNING: '%s' neither in running nor in imported env!\n", > vars[i]); + else > + printf("WARNING: '%s' not in imported env, deleting it! \n", vars[i]); > + } > + > debug("INSERT: done\n"); > return 1; /* everything OK */ > } Best regards, Marek Vasut
On 08/24/2012 04:58 PM, Marek Vasut wrote: > Dear Gerlando Falauto, > >> When variables explicitly specified on the command line are not present >> in the imported env, delete them from the running env. >> If the variable is also missing from the running env, issue a warning. >> >> Signed-off-by: Gerlando Falauto<gerlando.falauto@keymile.com> > > Whew! I made it through ... it wasn't that scary in the end ;-) > >> --- >> lib/hashtable.c | 48 +++++++++++++++++++++++++++++++++++++++++------- >> 1 file changed, 41 insertions(+), 7 deletions(-) >> >> diff --git a/lib/hashtable.c b/lib/hashtable.c >> index f3f47de..b3d0b64 100644 >> --- a/lib/hashtable.c >> +++ b/lib/hashtable.c >> @@ -607,22 +607,32 @@ ssize_t hexport_r(struct hsearch_data *htab, const >> char sep, * himport() >> */ >> >> -/* Check whether variable name is amongst vars[] */ >> -static int is_var_in_set(const char *name, int nvars, char * const vars[]) >> +/* >> + * Check whether variable 'name' is amongst vars[], >> + * and remove all instances by setting the pointer to NULL >> + */ >> +static int is_var_in_set(const char *name, int nvars, char * vars[]) >> { >> int i = 0; >> + int res = 0; >> >> /* No variables specified means process all of them */ >> if (nvars == 0) >> return 1; >> >> for (i = 0; i< nvars; i++) { >> - if (!strcmp(name, vars[i])) >> - return 1; >> + if (vars[i] == NULL) >> + continue; >> + /* If we found it, delete all of them */ >> + if (!strcmp(name, vars[i])) { >> + vars[i] = NULL; >> + res = 1; > > break here ? Nope, if we find it, we should delete all of them (see comment above). > >> + } >> } >> - debug("Skipping non-listed variable %s\n", name); >> + if (!res) >> + debug("Skipping non-listed variable %s\n", name); >> >> - return 0; >> + return res; >> } >> >> /* >> @@ -662,9 +672,11 @@ static int is_var_in_set(const char *name, int nvars, >> char * const vars[]) >> >> int himport_r(struct hsearch_data *htab, >> const char *env, size_t size, const char sep, int flag, >> - int nvars, char * const vars[], int do_apply) >> + int nvars, char * const __vars[], int do_apply) > > Two underscores are reserved, use something else ;-) Like... one? three? ;-) > >> { >> char *data, *sp, *dp, *name, *value; >> + char *vars[nvars]; >> + int i; >> >> /* Test for correct arguments. */ >> if (htab == NULL) { >> @@ -681,6 +693,10 @@ int himport_r(struct hsearch_data *htab, >> memcpy(data, env, size); >> dp = data; >> >> + /* make a local copy of the list of variables */ >> + if (nvars) >> + memcpy(vars, __vars, sizeof(__vars[0]) * nvars); >> + >> if ((flag& H_NOCLEAR) == 0) { >> /* Destroy old hash table if one exists */ >> debug("Destroy Hash Table: %p table = %p\n", htab, >> @@ -809,6 +825,24 @@ int himport_r(struct hsearch_data *htab, >> debug("INSERT: free(data = %p)\n", data); >> free(data); >> >> + /* process variables which were not considered */ >> + for (i = 0; i< nvars; i++) { >> + if (vars[i] == NULL) >> + continue; >> + /* >> + * All variables which were not deleted from the variable list >> + * were not present in the imported env >> + * This could mean two things: >> + * a) if the variable was present in current env, we delete it >> + * b) if the variable was not present in current env, we notify >> + * it might be a typo >> + */ >> + if (hdelete_r(vars[i], htab, do_apply) == 0) >> + printf("WARNING: '%s' neither in running nor in imported > env!\n", >> vars[i]); + else >> + printf("WARNING: '%s' not in imported env, deleting it! > \n", vars[i]); >> + } >> + >> debug("INSERT: done\n"); >> return 1; /* everything OK */ >> } > > Best regards, > Marek Vasut Best regards, Gerlando
Dear Gerlando Falauto, > On 08/24/2012 04:58 PM, Marek Vasut wrote: > > Dear Gerlando Falauto, > > > >> When variables explicitly specified on the command line are not present > >> in the imported env, delete them from the running env. > >> If the variable is also missing from the running env, issue a warning. > >> > >> Signed-off-by: Gerlando Falauto<gerlando.falauto@keymile.com> > > > > Whew! I made it through ... it wasn't that scary in the end ;-) > > > >> --- > >> > >> lib/hashtable.c | 48 > >> +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 41 > >> insertions(+), 7 deletions(-) > >> > >> diff --git a/lib/hashtable.c b/lib/hashtable.c > >> index f3f47de..b3d0b64 100644 > >> --- a/lib/hashtable.c > >> +++ b/lib/hashtable.c > >> @@ -607,22 +607,32 @@ ssize_t hexport_r(struct hsearch_data *htab, const > >> char sep, * himport() > >> > >> */ > >> > >> -/* Check whether variable name is amongst vars[] */ > >> -static int is_var_in_set(const char *name, int nvars, char * const > >> vars[]) +/* > >> + * Check whether variable 'name' is amongst vars[], > >> + * and remove all instances by setting the pointer to NULL > >> + */ > >> +static int is_var_in_set(const char *name, int nvars, char * vars[]) > >> > >> { > >> > >> int i = 0; > >> > >> + int res = 0; > >> > >> /* No variables specified means process all of them */ > >> if (nvars == 0) > >> > >> return 1; > >> > >> for (i = 0; i< nvars; i++) { > >> > >> - if (!strcmp(name, vars[i])) > >> - return 1; > >> + if (vars[i] == NULL) > >> + continue; > >> + /* If we found it, delete all of them */ > >> + if (!strcmp(name, vars[i])) { > >> + vars[i] = NULL; > >> + res = 1; > > > > break here ? > > Nope, if we find it, we should delete all of them (see comment above). Stupid me, of course now I see the logic! Sorry! > >> + } > >> > >> } > >> > >> - debug("Skipping non-listed variable %s\n", name); > >> + if (!res) > >> + debug("Skipping non-listed variable %s\n", name); > >> > >> - return 0; > >> + return res; > >> > >> } > >> > >> /* > >> > >> @@ -662,9 +672,11 @@ static int is_var_in_set(const char *name, int > >> nvars, char * const vars[]) > >> > >> int himport_r(struct hsearch_data *htab, > >> > >> const char *env, size_t size, const char sep, int flag, > >> > >> - int nvars, char * const vars[], int do_apply) > >> + int nvars, char * const __vars[], int do_apply) > > > > Two underscores are reserved, use something else ;-) > > Like... one? three? ;-) I think one is the way to go ... http://lwn.net/Articles/509149/ definitelly not like this ;-) [...] Best regards, Marek Vasut
On 08/24/2012 11:12 PM, Marek Vasut wrote: > Dear Gerlando Falauto, > >> On 08/24/2012 04:58 PM, Marek Vasut wrote: >>> Dear Gerlando Falauto, >>> >>>> When variables explicitly specified on the command line are not present >>>> in the imported env, delete them from the running env. >>>> If the variable is also missing from the running env, issue a warning. >>>> >>>> Signed-off-by: Gerlando Falauto<gerlando.falauto@keymile.com> >>> >>> Whew! I made it through ... it wasn't that scary in the end ;-) >>> >>>> --- >>>> >>>> lib/hashtable.c | 48 >>>> +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 41 >>>> insertions(+), 7 deletions(-) >>>> >>>> diff --git a/lib/hashtable.c b/lib/hashtable.c >>>> index f3f47de..b3d0b64 100644 >>>> --- a/lib/hashtable.c >>>> +++ b/lib/hashtable.c >>>> @@ -607,22 +607,32 @@ ssize_t hexport_r(struct hsearch_data *htab, const >>>> char sep, * himport() >>>> >>>> */ >>>> >>>> -/* Check whether variable name is amongst vars[] */ >>>> -static int is_var_in_set(const char *name, int nvars, char * const >>>> vars[]) +/* >>>> + * Check whether variable 'name' is amongst vars[], >>>> + * and remove all instances by setting the pointer to NULL >>>> + */ >>>> +static int is_var_in_set(const char *name, int nvars, char * vars[]) >>>> >>>> { >>>> >>>> int i = 0; >>>> >>>> + int res = 0; >>>> >>>> /* No variables specified means process all of them */ >>>> if (nvars == 0) >>>> >>>> return 1; >>>> >>>> for (i = 0; i< nvars; i++) { >>>> >>>> - if (!strcmp(name, vars[i])) >>>> - return 1; >>>> + if (vars[i] == NULL) >>>> + continue; >>>> + /* If we found it, delete all of them */ >>>> + if (!strcmp(name, vars[i])) { >>>> + vars[i] = NULL; >>>> + res = 1; >>> >>> break here ? >> >> Nope, if we find it, we should delete all of them (see comment above). > > Stupid me, of course now I see the logic! Sorry! > >>>> + } >>>> >>>> } >>>> >>>> - debug("Skipping non-listed variable %s\n", name); >>>> + if (!res) >>>> + debug("Skipping non-listed variable %s\n", name); >>>> >>>> - return 0; >>>> + return res; >>>> >>>> } >>>> >>>> /* >>>> >>>> @@ -662,9 +672,11 @@ static int is_var_in_set(const char *name, int >>>> nvars, char * const vars[]) >>>> >>>> int himport_r(struct hsearch_data *htab, >>>> >>>> const char *env, size_t size, const char sep, int flag, >>>> >>>> - int nvars, char * const vars[], int do_apply) >>>> + int nvars, char * const __vars[], int do_apply) >>> >>> Two underscores are reserved, use something else ;-) >> >> Like... one? three? ;-) > > I think one is the way to go ... http://lwn.net/Articles/509149/ definitelly not > like this ;-) > > [...] The way I understand it, two underscores are reserved for the compiler's internal use, whereas a single underscore is usually reserved for library function names. So I'm sending a v5 of this patch (not the whole set) with some new naming altogether (is_var_in_set also deserves some better naming as the new implementation would otherwise make it very misleading). Best regards, Gerlando
Dear Gerlando Falauto, In message <50379AD5.6030709@keymile.com> you wrote: > > >> int himport_r(struct hsearch_data *htab, > >> const char *env, size_t size, const char sep, int flag, > >> - int nvars, char * const vars[], int do_apply) > >> + int nvars, char * const __vars[], int do_apply) > > > > Two underscores are reserved, use something else ;-) > > Like... one? three? ;-) Neither of these. Use another name. Best regards, Wolfgang Denk
On 09/02/2012 02:01 PM, Wolfgang Denk wrote: > Dear Gerlando Falauto, > > In message<50379AD5.6030709@keymile.com> you wrote: >> >>>> int himport_r(struct hsearch_data *htab, >>>> const char *env, size_t size, const char sep, int flag, >>>> - int nvars, char * const vars[], int do_apply) >>>> + int nvars, char * const __vars[], int do_apply) >>> >>> Two underscores are reserved, use something else ;-) >> >> Like... one? three? ;-) > > Neither of these. Use another name. That was more of a joke. :-) Anyway, please see patch v5 (for this patch only). Thank you, Gerlando
diff --git a/lib/hashtable.c b/lib/hashtable.c index f3f47de..b3d0b64 100644 --- a/lib/hashtable.c +++ b/lib/hashtable.c @@ -607,22 +607,32 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep, * himport() */ -/* Check whether variable name is amongst vars[] */ -static int is_var_in_set(const char *name, int nvars, char * const vars[]) +/* + * Check whether variable 'name' is amongst vars[], + * and remove all instances by setting the pointer to NULL + */ +static int is_var_in_set(const char *name, int nvars, char * vars[]) { int i = 0; + int res = 0; /* No variables specified means process all of them */ if (nvars == 0) return 1; for (i = 0; i < nvars; i++) { - if (!strcmp(name, vars[i])) - return 1; + if (vars[i] == NULL) + continue; + /* If we found it, delete all of them */ + if (!strcmp(name, vars[i])) { + vars[i] = NULL; + res = 1; + } } - debug("Skipping non-listed variable %s\n", name); + if (!res) + debug("Skipping non-listed variable %s\n", name); - return 0; + return res; } /* @@ -662,9 +672,11 @@ static int is_var_in_set(const char *name, int nvars, char * const vars[]) int himport_r(struct hsearch_data *htab, const char *env, size_t size, const char sep, int flag, - int nvars, char * const vars[], int do_apply) + int nvars, char * const __vars[], int do_apply) { char *data, *sp, *dp, *name, *value; + char *vars[nvars]; + int i; /* Test for correct arguments. */ if (htab == NULL) { @@ -681,6 +693,10 @@ int himport_r(struct hsearch_data *htab, memcpy(data, env, size); dp = data; + /* make a local copy of the list of variables */ + if (nvars) + memcpy(vars, __vars, sizeof(__vars[0]) * nvars); + if ((flag & H_NOCLEAR) == 0) { /* Destroy old hash table if one exists */ debug("Destroy Hash Table: %p table = %p\n", htab, @@ -809,6 +825,24 @@ int himport_r(struct hsearch_data *htab, debug("INSERT: free(data = %p)\n", data); free(data); + /* process variables which were not considered */ + for (i = 0; i < nvars; i++) { + if (vars[i] == NULL) + continue; + /* + * All variables which were not deleted from the variable list + * were not present in the imported env + * This could mean two things: + * a) if the variable was present in current env, we delete it + * b) if the variable was not present in current env, we notify + * it might be a typo + */ + if (hdelete_r(vars[i], htab, do_apply) == 0) + printf("WARNING: '%s' neither in running nor in imported env!\n", vars[i]); + else + printf("WARNING: '%s' not in imported env, deleting it!\n", vars[i]); + } + debug("INSERT: done\n"); return 1; /* everything OK */ }
When variables explicitly specified on the command line are not present in the imported env, delete them from the running env. If the variable is also missing from the running env, issue a warning. Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com> --- lib/hashtable.c | 48 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 41 insertions(+), 7 deletions(-)