{"thread":{"id":"41517","subject":"[PATCH] Change type of signed int flags to unsigned","startedAt":"2016-02-25T13:24:32Z","lastAt":"2016-02-28T07:59:20Z","messageCount":2,"participants":["Saurav Sachidanand","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"279335","messageId":"1456406672-5661-1-git-send-email-sauravsachidanand@gmail.com","threadId":"41517","inReplyTo":null,"subject":"[PATCH] Change type of signed int flags to unsigned","fromName":"Saurav Sachidanand","fromEmail":"sauravsachidanand@gmail.com","sentAt":"2016-02-25T13:24:32Z","receivedAt":"2016-02-25T13:24:32Z","isPatch":true,"sender":{"key":"sauravsachidanand@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12742708?v=4"},"body":"“pattern” and “exclude” are two structs defined in attr.c and dir.h\nrespectively. Each contains a field named “flags” of type int, that\ntakes on values from the set of positive integers {1, 4, 8, 16}\nenumerated through the macro EXC_FLAG_*.\n\nThat the most significant bit (used to store the sign) of these two\nfields is not used in any special way, is observed from the fact\nthat, the flags fields (accessed within attr.c, dir.c, and\nbuiltin/check-ignore.c) is either checked for it's value using the &\noperator (e.g.: flags & EXC_FLAG_NODIR), or assigned a value of 0\nfirst and then assigned any one of {1, 4, 8, 16} using the | operator\n(e.g.: flags |= EXC_FLAG_NODIR). Hence, change the type of flags\nto unsigned in both structs.\n\nFurthermore, flags is passed by reference to the function\nparse_exclude_pattern defined in dir.c, which accepts an “int *” type\nfor the flags argument. To avoid converting between pointers to\ndifferent types, change type of the flags argument to “unsigned *”.\n\nWhile we’re at it, document the flags field of exclude to explicitly\nstate the values it’s supposed to take on.\n\nSigned-off-by: Saurav Sachidanand <sauravsachidanand@gmail.com>\n---\n attr.c | 2 +-\n dir.c  | 4 ++--\n dir.h  | 4 ++--\n 3 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 086c08d..679e13c 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -124,7 +124,7 @@ struct pattern {\n \tconst char *pattern;\n \tint patternlen;\n \tint nowildcardlen;\n-\tint flags;\t\t/* EXC_FLAG_* */\n+\tunsigned flags;\t\t/* EXC_FLAG_* */\n };\n\n /*\ndiff --git a/dir.c b/dir.c\nindex 552af23..d36fda7 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -459,7 +459,7 @@ int no_wildcard(const char *string)\n\n void parse_exclude_pattern(const char **pattern,\n \t\t\t   int *patternlen,\n-\t\t\t   int *flags,\n+\t\t\t   unsigned *flags,\n \t\t\t   int *nowildcardlen)\n {\n \tconst char *p = *pattern;\n@@ -500,7 +500,7 @@ void add_exclude(const char *string, const char *base,\n {\n \tstruct exclude *x;\n \tint patternlen;\n-\tint flags;\n+\tunsigned flags;\n \tint nowildcardlen;\n\n \tparse_exclude_pattern(&string, &patternlen, &flags, &nowildcardlen);\ndiff --git a/dir.h b/dir.h\nindex 3ec3fb0..e34df5e 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -28,7 +28,7 @@ struct exclude {\n \tint nowildcardlen;\n \tconst char *base;\n \tint baselen;\n-\tint flags;\n+\tunsigned flags;\t\t/* EXC_FLAG_* */\n\n \t/*\n \t * Counting starts from 1 for line numbers in ignore files,\n@@ -244,7 +244,7 @@ extern struct exclude_list *add_exclude_list(struct dir_struct *dir,\n extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,\n \t\t\t\t\t  struct exclude_list *el, int check_index);\n extern void add_excludes_from_file(struct dir_struct *, const char *fname);\n-extern void parse_exclude_pattern(const char **string, int *patternlen, int *flags, int *nowildcardlen);\n+extern void parse_exclude_pattern(const char **string, int *patternlen, unsigned *flags, int *nowildcardlen);\n extern void add_exclude(const char *string, const char *base,\n \t\t\tint baselen, struct exclude_list *el, int srcpos);\n extern void clear_exclude_list(struct exclude_list *el);\n--\n2.7.1.339.g0233b80\n\nThis patch is for the suggested microproject for GSoC 2016 titled\n\"Use unsigned integral type for collection of bits.\"\n"},{"id":"279700","messageId":"CAPig+cTeo=7GJNkNu8WwGYE2pDH51U_qeOaa65jx2k_ri45pQA@mail.gmail.com","threadId":"41517","inReplyTo":"1456406672-5661-1-git-send-email-sauravsachidanand@gmail.com","subject":"Re: [PATCH] Change type of signed int flags to unsigned","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-28T07:59:20Z","receivedAt":"2016-02-28T07:59:20Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Feb 25, 2016 at 8:24 AM, Saurav Sachidanand\n<sauravsachidanand@gmail.com> wrote:\n> “pattern” and “exclude” are two structs defined in attr.c and dir.h\n> respectively. Each contains a field named “flags” of type int, that\n> takes on values from the set of positive integers {1, 4, 8, 16}\n> enumerated through the macro EXC_FLAG_*.\n>\n> That the most significant bit (used to store the sign) of these two\n> fields is not used in any special way, is observed from the fact\n> that, the flags fields (accessed within attr.c, dir.c, and\n> builtin/check-ignore.c) is either checked for it's value using the &\n> operator (e.g.: flags & EXC_FLAG_NODIR), or assigned a value of 0\n> first and then assigned any one of {1, 4, 8, 16} using the | operator\n> (e.g.: flags |= EXC_FLAG_NODIR). Hence, change the type of flags\n> to unsigned in both structs.\n>\n> Furthermore, flags is passed by reference to the function\n> parse_exclude_pattern defined in dir.c, which accepts an “int *” type\n> for the flags argument. To avoid converting between pointers to\n> different types, change type of the flags argument to “unsigned *”.\n\nIf you follow Duy's suggestion[1] of checking for additional\nsign-conversion warnings and fixing the additional instances, then\nthere will be a bit more fallout than this one function. In that case,\nyou might consider changing this paragraph to be a bit more generic;\nfor instance, perhaps something like this:\n\n    These \"flags\" fields are passed to several functions, so convert\n    the functions to accept 'unsigned' rather than 'int', as well.\n\n> While we’re at it, document the flags field of exclude to explicitly\n> state the values it’s supposed to take on.\n\nGood.\n\n> Signed-off-by: Saurav Sachidanand <sauravsachidanand@gmail.com>\n> ---\n\nRight here below the \"---\" line is where you'd write commentary about\nthe patch. In this case, you should mention that this is a GSoC\nmicroproject.\n\nAs an aid to reviewers, also provide a link to the previous version\n(like this [2]), and explain what changed since that attempt. In this\ncase, you refined the commit message and used bare \"unsigned\" in the\npatch rather than \"unsigned int\".\n\nThe patch itself looks reasonable (though it would look better if\nadditional fixes were made as suggested by Duy[1]).\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/286821/focus=286900\n[2]: http://thread.gmane.org/gmane.comp.version-control.git/286821\n\n>  attr.c | 2 +-\n>  dir.c  | 4 ++--\n>  dir.h  | 4 ++--\n>  3 files changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/attr.c b/attr.c\n> index 086c08d..679e13c 100644\n> --- a/attr.c\n> +++ b/attr.c\n> @@ -124,7 +124,7 @@ struct pattern {\n>         const char *pattern;\n>         int patternlen;\n>         int nowildcardlen;\n> -       int flags;              /* EXC_FLAG_* */\n> +       unsigned flags;         /* EXC_FLAG_* */\n>  };\n>\n>  /*\n> diff --git a/dir.c b/dir.c\n> index 552af23..d36fda7 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -459,7 +459,7 @@ int no_wildcard(const char *string)\n>\n>  void parse_exclude_pattern(const char **pattern,\n>                            int *patternlen,\n> -                          int *flags,\n> +                          unsigned *flags,\n>                            int *nowildcardlen)\n>  {\n>         const char *p = *pattern;\n> @@ -500,7 +500,7 @@ void add_exclude(const char *string, const char *base,\n>  {\n>         struct exclude *x;\n>         int patternlen;\n> -       int flags;\n> +       unsigned flags;\n>         int nowildcardlen;\n>\n>         parse_exclude_pattern(&string, &patternlen, &flags, &nowildcardlen);\n> diff --git a/dir.h b/dir.h\n> index 3ec3fb0..e34df5e 100644\n> --- a/dir.h\n> +++ b/dir.h\n> @@ -28,7 +28,7 @@ struct exclude {\n>         int nowildcardlen;\n>         const char *base;\n>         int baselen;\n> -       int flags;\n> +       unsigned flags;         /* EXC_FLAG_* */\n>\n>         /*\n>          * Counting starts from 1 for line numbers in ignore files,\n> @@ -244,7 +244,7 @@ extern struct exclude_list *add_exclude_list(struct dir_struct *dir,\n>  extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,\n>                                           struct exclude_list *el, int check_index);\n>  extern void add_excludes_from_file(struct dir_struct *, const char *fname);\n> -extern void parse_exclude_pattern(const char **string, int *patternlen, int *flags, int *nowildcardlen);\n> +extern void parse_exclude_pattern(const char **string, int *patternlen, unsigned *flags, int *nowildcardlen);\n>  extern void add_exclude(const char *string, const char *base,\n>                         int baselen, struct exclude_list *el, int srcpos);\n>  extern void clear_exclude_list(struct exclude_list *el);\n> --\n> 2.7.1.339.g0233b80\n"}]}