[PATCH][GSoc] Changed signed flags to unsigned type

[PATCH][GSoc] Changed signed flags to unsigned type

[Vedant Bassi]

As part of my microproject :

Use unsigned integral type for collection of bits:
Pick one field of a structure that (1) is of signed integral type and
(2) is used as a collection of multiple bits. Discuss if there is a
good reason why it has to be a signed integral field and change it to
an unsigned type otherwise.

More ref: https://public-inbox.org/git/xmqqsiebrlez.fsf@gitster.dls.corp.google.com
http://stackoverflow.com/questions/29795170/usage-of-signed-vs-unsigned-variables-for-flags-in-c

I have found several structures where a signed int was used on flags
for bitwise & to check various cases.


diff --git a/bisect.h b/bisect.h

index a979a7f..4b562a8 100644

--- a/bisect.h

+++ b/bisect.h

@@ -16,6 +16,8 @@ extern struct commit_list *filter_skipped(struct
commit_list *list,



 struct rev_list_info {

        struct rev_info *revs;

+

+ // int flags changed to unsigned int

        unsigned int flags;

        int show_timestamp;

        int hdr_termination;

diff --git a/builtin/add.c b/builtin/add.c

index 9f53f02..1212eea 100644

--- a/builtin/add.c

+++ b/builtin/add.c

@@ -26,6 +26,8 @@ static int patch_interactive, add_interactive,
edit_interactive;

 static int take_worktree_changes;



 struct update_callback_data {

+

+ // flags supposed to be unsigned

        int flags;

        int add_errors;

 };

diff --git a/parse-options.h b/parse-options.h

index dcd8a09..ad180c9 100644

--- a/parse-options.h

+++ b/parse-options.h

@@ -107,7 +107,9 @@ struct option {

        const char *argh;

        const char *help;



-   int flags;

+ // int flags changed to unsigned

+

+ unsigned int flags;

        parse_opt_cb *callback;

        intptr_t defval;

 };

@@ -201,7 +203,10 @@ struct parse_opt_ctx_t {

        const char **out;

        int argc, cpidx, total;

        const char *opt;

-   int flags;

+

+ // int flags changed to unsigned

+

+ unsigned int flags;

        const char *prefix;

 };



-------

result : the changes were made in bisect.h , parse-options.h and  builtin/add.c

I have not yet  tested these changes.

Re: [PATCH][GSoc] Changed signed flags to unsigned type

[Christian Couder]

On Thu, Mar 9, 2017 at 5:24 AM, Vedant Bassi <sharababy.dev@gmail.com> wrote:
> As part of my microproject :
>
> Use unsigned integral type for collection of bits:
> Pick one field of a structure that (1) is of signed integral type and
> (2) is used as a collection of multiple bits. Discuss if there is a
> good reason why it has to be a signed integral field and change it to
> an unsigned type otherwise.
>
> More ref: https://public-inbox.org/git/xmqqsiebrlez.fsf@gitster.dls.corp.google.com
> http://stackoverflow.com/questions/29795170/usage-of-signed-vs-unsigned-variables-for-flags-in-c
>
> I have found several structures where a signed int was used on flags
> for bitwise & to check various cases.

This email has a title that starts with "[PATCH]" but it doesn't
contain a real patch that can be applied using `git am` for example.
Please look at https://git.github.io/SoC-2017-Microprojects/ and at
Documentation/SubmittingPatches about how patches should be created.

> diff --git a/bisect.h b/bisect.h
>
> index a979a7f..4b562a8 100644
>
> --- a/bisect.h
>
> +++ b/bisect.h
>
> @@ -16,6 +16,8 @@ extern struct commit_list *filter_skipped(struct
> commit_list *list,
>
>
>
>  struct rev_list_info {
>
>         struct rev_info *revs;
>
> +
>
> + // int flags changed to unsigned int

You don't need to add such comments as "what has been done" will be
obvious when looking at the commit. It could be interesting to explain
"why the change is made" in the commit message though.

If there is something subtle in the code or something that could save
a reader some time if it was documented, then a comment might be
useful, but anyway comments should use "/* ... */" markers, not "//".

>         unsigned int flags;
>
>         int show_timestamp;
>
>         int hdr_termination;

[...]

> result : the changes were made in bisect.h , parse-options.h and  builtin/add.c
>
> I have not yet  tested these changes.

I think sending just one patch for bisect.h is ok. If you really want
you could send another patch for parse-options.h and yet another one
for builtin/add.c, all in the same patch series.

Anyway please test that the patches can be applied (using git am) and
that they look good (compared with other commits) when applied before
sending them to the list. And yeah it is also a good idea to also
check that the test suite still passes after each patch before sending
them.