{"thread":{"id":"10970","subject":"[PATCH] Simplify the code and avoid an attribution.","startedAt":"2007-11-22T01:00:02Z","lastAt":"2007-11-22T20:29:11Z","messageCount":3,"participants":["André Goddard Rosa","Mike Hommey","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"60589","messageId":"b8bf37780711211700m1aa32006t6302f134f9c3fb7e@mail.gmail.com","threadId":"10970","inReplyTo":null,"subject":"[PATCH] Simplify the code and avoid an attribution.","fromName":"André Goddard Rosa","fromEmail":"andre.goddard@gmail.com","sentAt":"2007-11-22T01:00:02Z","receivedAt":"2007-11-22T01:00:02Z","isPatch":true,"sender":{"key":"andre.goddard@gmail.com","avatar":null},"body":"Hi, all!\n\n    Please cc: me as I'm not subscribed. I'm sending the patch inline\nonly for review, probably it is mangled.\n    Please use the attached patch if you agree with it. Sorry about\nsending it attached.\n\n>From cd0cc6995684e2801011910735146052e5b59ccc Mon Sep 17 00:00:00 2001\nFrom: Andre Goddard Rosa <andre.goddard@gmail.com>\nDate: Tue, 27 Nov 2007 10:22:46 -0200\nSubject: [PATCH] Simplify the code and avoid an attribution.\n\nSigned-off-by: Andre Goddard Rosa <andre.goddard@gmail.com>\n---\n config.c          |   19 ++++++++++---------\n mailmap.c         |    2 +-\n xdiff-interface.c |    2 +-\n 3 files changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 56e99fc..7c9fcdd 100644\n--- a/config.c\n+++ b/config.c\n@@ -447,15 +447,16 @@ int git_config_from_file(config_fn_t fn, const\nchar *filename)\n \tint ret;\n \tFILE *f = fopen(filename, \"r\");\n\n-\tret = -1;\n-\tif (f) {\n-\t\tconfig_file = f;\n-\t\tconfig_file_name = filename;\n-\t\tconfig_linenr = 1;\n-\t\tret = git_parse_file(fn);\n-\t\tfclose(f);\n-\t\tconfig_file_name = NULL;\n-\t}\n+\tif (!f)\n+\t\treturn -1;\n+\n+\tconfig_file = f;\n+\tconfig_file_name = filename;\n+\tconfig_linenr = 1;\n+\tret = git_parse_file(fn);\n+\tfclose(f);\n+\tconfig_file_name = NULL;\n+\n \treturn ret;\n }\n\ndiff --git a/mailmap.c b/mailmap.c\nindex 8714167..0c13ecd 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -7,7 +7,7 @@ int read_mailmap(struct path_list *map, const char\n*filename, char **repo_abbrev\n \tchar buffer[1024];\n \tFILE *f = fopen(filename, \"r\");\n\n-\tif (f == NULL)\n+\tif (!f)\n \t\treturn 1;\n \twhile (fgets(buffer, sizeof(buffer), f) != NULL) {\n \t\tchar *end_of_name, *left_bracket, *right_bracket;\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex be866d1..9dd1f3b 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -111,7 +111,7 @@ int read_mmfile(mmfile_t *ptr, const char *filename)\n\n \tif (stat(filename, &st))\n \t\treturn error(\"Could not stat %s\", filename);\n-\tif ((f = fopen(filename, \"rb\")) == NULL)\n+\tif (!(f = fopen(filename, \"rb\")))\n \t\treturn error(\"Could not open %s\", filename);\n \tsz = xsize_t(st.st_size);\n \tptr->ptr = xmalloc(sz);\n-- \n1.5.3.6.861.gd794-dirty\n\n\nFrom cd0cc6995684e2801011910735146052e5b59ccc Mon Sep 17 00:00:00 2001\nFrom: Andre Goddard Rosa <andre.goddard@gmail.com>\nDate: Tue, 27 Nov 2007 10:22:46 -0200\nSubject: [PATCH] Simplify the code and avoid an attribution.\n\nSigned-off-by: Andre Goddard Rosa <andre.goddard@gmail.com>\n---\n config.c          |   19 ++++++++++---------\n mailmap.c         |    2 +-\n xdiff-interface.c |    2 +-\n 3 files changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 56e99fc..7c9fcdd 100644\n--- a/config.c\n+++ b/config.c\n@@ -447,15 +447,16 @@ int git_config_from_file(config_fn_t fn, const char *filename)\n \tint ret;\n \tFILE *f = fopen(filename, \"r\");\n \n-\tret = -1;\n-\tif (f) {\n-\t\tconfig_file = f;\n-\t\tconfig_file_name = filename;\n-\t\tconfig_linenr = 1;\n-\t\tret = git_parse_file(fn);\n-\t\tfclose(f);\n-\t\tconfig_file_name = NULL;\n-\t}\n+\tif (!f)\n+\t\treturn -1;\n+\n+\tconfig_file = f;\n+\tconfig_file_name = filename;\n+\tconfig_linenr = 1;\n+\tret = git_parse_file(fn);\n+\tfclose(f);\n+\tconfig_file_name = NULL;\n+\n \treturn ret;\n }\n \ndiff --git a/mailmap.c b/mailmap.c\nindex 8714167..0c13ecd 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -7,7 +7,7 @@ int read_mailmap(struct path_list *map, const char *filename, char **repo_abbrev\n \tchar buffer[1024];\n \tFILE *f = fopen(filename, \"r\");\n \n-\tif (f == NULL)\n+\tif (!f)\n \t\treturn 1;\n \twhile (fgets(buffer, sizeof(buffer), f) != NULL) {\n \t\tchar *end_of_name, *left_bracket, *right_bracket;\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex be866d1..9dd1f3b 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -111,7 +111,7 @@ int read_mmfile(mmfile_t *ptr, const char *filename)\n \n \tif (stat(filename, &st))\n \t\treturn error(\"Could not stat %s\", filename);\n-\tif ((f = fopen(filename, \"rb\")) == NULL)\n+\tif (!(f = fopen(filename, \"rb\")))\n \t\treturn error(\"Could not open %s\", filename);\n \tsz = xsize_t(st.st_size);\n \tptr->ptr = xmalloc(sz);\n-- \n1.5.3.6.861.gd794-dirty\n\n"},{"id":"60704","messageId":"20071122200157.GC19675@glandium.org","threadId":"10970","inReplyTo":"b8bf37780711211700m1aa32006t6302f134f9c3fb7e@mail.gmail.com","subject":"Re: [PATCH] Simplify the code and avoid an attribution.","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2007-11-22T20:01:57Z","receivedAt":"2007-11-22T20:01:57Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Wed, Nov 21, 2007 at 11:00:02PM -0200, André Goddard Rosa wrote:\n> --- a/config.c\n> +++ b/config.c\n> @@ -447,15 +447,16 @@ int git_config_from_file(config_fn_t fn, const\n> char *filename)\n>  \tint ret;\n>  \tFILE *f = fopen(filename, \"r\");\n> \n> -\tret = -1;\n> -\tif (f) {\n> -\t\tconfig_file = f;\n> -\t\tconfig_file_name = filename;\n> -\t\tconfig_linenr = 1;\n> -\t\tret = git_parse_file(fn);\n> -\t\tfclose(f);\n> -\t\tconfig_file_name = NULL;\n> -\t}\n> +\tif (!f)\n> +\t\treturn -1;\n> +\n> +\tconfig_file = f;\n> +\tconfig_file_name = filename;\n> +\tconfig_linenr = 1;\n> +\tret = git_parse_file(fn);\n> +\tfclose(f);\n> +\tconfig_file_name = NULL;\n> +\n>  \treturn ret;\n>  }\n\nActually, since it is more likely that the file has been opened, the\noriginal code is more optimal because it doesn't generate a jump in most\ncases. And if you're worried about the ret variable, don't worry, it's\nmost likely stripped out by the compiler optimizations.\n\n> -\tif (f == NULL)\n> +\tif (!f)\n\n> -\tif ((f = fopen(filename, \"rb\")) == NULL)\n> +\tif (!(f = fopen(filename, \"rb\")))\n\nIt's a matter of taste\n\nMike\n"},{"id":"60707","messageId":"7vk5oa3tyg.fsf@gitster.siamese.dyndns.org","threadId":"10970","inReplyTo":"20071122200157.GC19675@glandium.org","subject":"Re: [PATCH] Simplify the code and avoid an attribution.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-22T20:29:11Z","receivedAt":"2007-11-22T20:29:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey <mh@glandium.org> writes:\n\n> On Wed, Nov 21, 2007 at 11:00:02PM -0200, André Goddard Rosa wrote:\n>> --- a/config.c\n>> +++ b/config.c\n>> @@ -447,15 +447,16 @@ int git_config_from_file(config_fn_t fn, const\n>> char *filename)\n>>  \tint ret;\n>>  \tFILE *f = fopen(filename, \"r\");\n>> \n>> -\tret = -1;\n>> -\tif (f) {\n>> -\t\tconfig_file = f;\n>> -\t\tconfig_file_name = filename;\n>> -\t\tconfig_linenr = 1;\n>> -\t\tret = git_parse_file(fn);\n>> -\t\tfclose(f);\n>> -\t\tconfig_file_name = NULL;\n>> -\t}\n>> +\tif (!f)\n>> +\t\treturn -1;\n>> +\n>> +\tconfig_file = f;\n>> +\tconfig_file_name = filename;\n>> +\tconfig_linenr = 1;\n>> +\tret = git_parse_file(fn);\n>> +\tfclose(f);\n>> +\tconfig_file_name = NULL;\n>> +\n>>  \treturn ret;\n>>  }\n>\n> Actually, since it is more likely that the file has been opened, the\n> original code is more optimal because it doesn't generate a jump in most\n> cases. And if you're worried about the ret variable, don't worry, it's\n> most likely stripped out by the compiler optimizations.\n\nI did not comment on this patch partly because I did not know\nwhat \"attribution\" meant.  I think it is a good change from\nreadability, not micro-optimization, point of view.\n\nIf you structure your code this way,\n\n\tdo this\n        if (there is an error)\n        \treturn error code;\n\tdo that\n\tdo the rest\n\nit is much easier to read than\n\n\tdo this\n        set error code pessimistically\n        if (it was successful) {\n        \tdo that\n                update error code\n                do the rest\n\t}\n        return error code\n\nespecially the more likely part that runs the real processing\n(\"do that...do the rest\") tends to grow over time, and even\nacquire other error checks.\n"}]}