Web lists-archives.com

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




On Thu, Mar 9, 2017 at 5:24 AM, Vedant Bassi <sharababy.dev@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxx
> 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.