Hi Gerlando,
On Mon, Apr 2, 2012 at 1:43 PM, Gerlando Falauto gerlando.falauto@keymile.com wrote:
On 04/02/2012 08:57 PM, Marek Vasut wrote:
Dear Gerlando Falauto,
Add 2 new arguments to himport_r():
o "nvars", "vars": number and list of variables to take into account (0 means ALL)
Looks nice and clean. But man, these patches are tough to review (not because you made bad job, just because they're hard code), lot of headache from them ;-)
I certainly agree with that - I still remember this from last time :-)
Tell me about it. I've been reworking them several times, usually after several weeks from posting them. Long enough to have forgotten everything, and to give you a bad headache. So I really appreciate your prompt feedback, thanks!
Gerlando
P.S. For the whole process I am using interactive rebase, which is painful enough. If anyone knows of any other way to "patch the patches", some advice would be very much appreciated... :-)
I don't know if this helps, but I'll say it anyway. I do a lot of this :-)
Normally if there is a lot of stuff I mess around with the patches on the top of the branch, with a 'wip' commit. That way I can test easily. I often have setup commits before the series and a wip one on top (patman makes it easy to skip these when sending to the mailing list). Then when I am ready to put the new changes back into the correct commits, I do a 'git rebase -i' and edit each commit.
For each commit I diff if against the top of the branch but only on files changed by the *current* commit. So in the case where 25 files were changed, but only 2 were mentioned in this commit, it is likely that I only want to bring over changes in those two files into the current commit.
I have an alias that does this:
function next_commit_changes() { out=$(git log --numstat --pretty=format: -n1 | tail -n+2) echo "$out" | awk '{printf("%s ", $3)}' }
function diff_branch() { echo "Changes in this commit:" echo git log --stat --oneline -n1
echo echo "Now performing diff against branch on those files only"
# print a stat list first echo $(next_commit_changes) git diff $1 --stat -- $(next_commit_changes)
# now run meld on each diff git diff $1 -- $(next_commit_changes) }
# db <branch>
# this is for splitting a large commit into multiple ones
# Usage: # git checkout mmc2 # git rebase -i HEAD~4 # on a commit use: # db mmc2 # this will print a list of changes to the files used by THIS commit # which have been made at mmc2 alias db='diff_branch'
Basically I then just use the arrows in meld to move over the changes I made into the right commit. Then 'git add -u' and 'git rebase --continue' and a line or two to the commit change log. Although I actually have two character aliases for nearly everything now...
Regards, Simon
NOTE: This patch does not change the current behaviour.
Signed-off-by: Gerlando Falautogerlando.falauto@keymile.com
common/cmd_nvedit.c | 3 ++- common/env_common.c | 6 ++++-- include/search.h | 6 +++++- lib/hashtable.c | 27 ++++++++++++++++++++++++++- 4 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index e762e76..59668a6 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -930,7 +930,8 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, addr = (char *)ep->data; }
- if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR) ==
- {
- if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
- 0, NULL) == 0) {
error("Environment import failed: errno = %d\n", errno); return 1; } diff --git a/common/env_common.c b/common/env_common.c index c33d22d..a93c062 100644 --- a/common/env_common.c +++ b/common/env_common.c @@ -182,7 +182,8 @@ void set_default_env(const char *s) }
if (himport_r(&env_htab, (char *)default_environment,
- sizeof(default_environment), '\0', 0) == 0)
- sizeof(default_environment), '\0', 0,
- 0, NULL) == 0)
error("Environment import failed: errno = %d\n", errno);
gd->flags |= GD_FLG_ENV_READY; @@ -207,7 +208,8 @@ int env_import(const char *buf, int check) } }
- if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0)) {
- if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
- 0, NULL)) {
gd->flags |= GD_FLG_ENV_READY; return 1; } diff --git a/include/search.h b/include/search.h index a4a5ef4..94d75fc 100644 --- a/include/search.h +++ b/include/search.h @@ -94,9 +94,13 @@ extern ssize_t hexport_r(struct hsearch_data *__htab, const char __sep, char **__resp, size_t __size, int argc, char * const argv[]);
+/*
- nvars: length of vars array
- vars: array of strings (variable names) to import (nvars == 0 means
all) + */ extern int himport_r(struct hsearch_data *__htab, const char *__env, size_t __size, const char __sep,
- int __flag);
- int __flag, int nvars, char * const vars[]);
/* Flags for himport_r() */ #define H_NOCLEAR (1<< 0) /* do not clear hash table before
importing */
diff --git a/lib/hashtable.c b/lib/hashtable.c index abd61c8..0610e86 100644 --- a/lib/hashtable.c +++ b/lib/hashtable.c @@ -603,6 +603,24 @@ 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[]) +{
- int i = 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;
- }
- debug("Skipping non-listed variable %s\n", name);
- return 0;
+}
/* * Import linearized data into hash table. * @@ -639,7 +657,8 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep, */
int himport_r(struct hsearch_data *htab,
- const char *env, size_t size, const char sep, int flag)
- const char *env, size_t size, const char sep, int flag,
- int nvars, char * const vars[])
{ char *data, *sp, *dp, *name, *value;
@@ -726,6 +745,8 @@ int himport_r(struct hsearch_data *htab, *dp++ = '\0'; /* terminate name */
debug("DELETE CANDIDATE: "%s"\n", name);
- if (!is_var_in_set(name, nvars, vars))
- continue;
if (hdelete_r(name, htab) == 0) debug("DELETE ERROR
##############################\n");
@@ -743,6 +764,10 @@ int himport_r(struct hsearch_data *htab, *sp++ = '\0'; /* terminate value */ ++dp;
- /* Skip variables which are not supposed to be processed
*/
- if (!is_var_in_set(name, nvars, vars))
- continue;
/* enter into hash table */ e.key = name; e.data = value;