Re: [PATCH] git: use U to denote unsigned to prevent UB

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

 



Hi,

Seija Kijin wrote:

> 1 << can be UB if 1 ends up overflowing and
> being assigned to an unsigned int or long.
>
> Signed-off-by: Seija Kijin <doremylover123@xxxxxxxxx>
> ---
>  builtin/checkout.c     |  2 +-
>  builtin/merge-tree.c   |  4 ++--
>  builtin/receive-pack.c |  2 +-
>  color.c                |  4 ++--
>  delta-islands.c        |  2 +-
>  diff-delta.c           |  2 +-
>  diff.c                 |  2 +-
>  help.c                 |  2 +-
>  imap-send.c            |  2 +-
>  merge-ort.c            | 18 +++++++++---------
>  xdiff/xhistogram.c     |  2 +-
>  xdiff/xprepare.c       |  4 ++--
>  12 files changed, 23 insertions(+), 23 deletions(-)

That said, most of these don't overflow, so it's not obvious this
results in higher quality or more readable code than before the patch.

By "not obvious" I don't mean that it _doesn't_, by the way, but just
that we don't have enough information to evaluate it here.  What
motivated writing this patch?  Is there a style guideline about it
that will remind us not to backslide in the future, for example?  Or
is there a tool that notices?  Was there an example you ran into that
led you to look for more examples?

This kind of information about context will make it easier for other
in the project to ensure the patch does what it intends, and even more
importantly, to see if there are additional checks to add or other
instances that also need updating.

Thanks and hope that helps,
Jonathan




[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]

  Powered by Linux