Re: RFC: C code cleanup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks. The ones I don't comment on below looked like sensible changes
(except that their log messages need to conform to the project's
style).

> diff --git a/test-date.c b/test-date.c
> index 6bcd5b0..42642ed 100644
> --- a/test-date.c
> +++ b/test-date.c
> @@ -16,7 +16,7 @@ static void show_dates(char **argv, struct timeval *now)
> Â Â Â Â}
> Â}
>
> -static void parse_dates(char **argv, struct timeval *now)
> +static void parse_dates(char **argv)
> Â{

As parse-dates mode does not handle any "relative" dates now, the
parameter happens not to be used in today's code. But judging from the
caller and other callers in the same if/else cascade, it probably is
better to leave this as-is.

> @@ -61,7 +61,7 @@ int main(int argc, char **argv)
> Â Â Â Âif (!strcmp(*argv, "show"))
> Â Â Â Â Â Â Â Âshow_dates(argv+1, &now);
> Â Â Â Âelse if (!strcmp(*argv, "parse"))
> - Â Â Â Â Â Â Â parse_dates(argv+1, &now);
> + Â Â Â Â Â Â Â parse_dates(argv+1);
> Â Â Â Âelse if (!strcmp(*argv, "approxidate"))
> Â Â Â Â Â Â Â Âparse_approxidate(argv+1, &now);
> Â Â Â Âelse

> commit 151ad9c45f08aa81598664e6e198af881fe52b77
> Author: Andy Lester <andy@xxxxxxxxxxxx>
> Date: Â Wed Jun 1 23:16:10 2011 -0500
>
> Â ÂRemoved unnecessary test in fuzzy_matchlines(). size_t can never be negative
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 530d4bb..7e6fa4d 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -250,9 +250,6 @@ static int fuzzy_matchlines(const char *s1, size_t n1,
> Â Â Â Âconst char *last2 = s2 + n2 - 1;
> Â Â Â Âint result = 0;
>
> - Â Â Â if (n1 < 0 || n2 < 0)
> - Â Â Â Â Â Â Â return 0;
> -

Looks like the author wanted to assert() to make sure the length
adjustments were not screwed up (a possible bug can be to update *.len
by subtracting too many, causing it to wrap around to become a
negative value but because size_t is unsigned, this is a wrong test to
catch it).

Perhaps casting these to ssize_t and dying with die("BUG: len
adjustment error") would be a better fix that is more faithful to the
intention of the original.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]