All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: "René Scharfe" <l.s.r@web.de>, "Git List" <git@vger.kernel.org>
Subject: Re: [PATCH] mergesort: avoid left shift overflow
Date: Wed, 17 Nov 2021 01:19:08 -0800	[thread overview]
Message-ID: <xmqqr1bf2l83.fsf@gitster.g> (raw)
In-Reply-To: nycvar.QRO.7.76.6.2111161505500.21127@tvgsbejvaqbjf.bet

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> diff --git a/mergesort.c b/mergesort.c
>> index 6216835566..bd9c6ef8ee 100644
>> --- a/mergesort.c
>> +++ b/mergesort.c
>> @@ -63,7 +63,7 @@ void *llist_mergesort(void *list,
>>  		void *next = get_next_fn(list);
>>  		if (next)
>>  			set_next_fn(list, NULL);
>> -		for (i = 0; n & (1 << i); i++)
>> +		for (i = 0; n & ((size_t)1 << i); i++)
>
> I was a bit concerned about the operator precedence (some of which I
> remember by heart, some not), but according to
> https://en.cppreference.com/w/c/language/operator_precedence the cast has
> a higher precedence than the shift operator.
>
> I would have preferred an extra pair of parentheses around `(size_t)1` so
> that I (and other readers) do not have to remember or look up the operator
> precedence, but it _is_ correct.

Interesting.

I do not quite see the need for it myself, but if we wanted to, we
can smoke them out with this, I think.

	$ cat >contrib/coccinelle/cast.cocci <<-\EOF
	@@
	type T;
	expression V, C;
	@@
	-(T) V << C
	+((T) V) << C
	EOF
	$ make contrib/coccinelle/cast.cocci.patch
	$ git apply --stat contrib/coccinelle/cast.cocci.patch
         compat/mingw.c     |    2 +-
         compat/mingw.c     |    2 +-
         ewah/bitmap.c      |    2 +-
         ewah/ewok_rlw.h    |    6 +++---
         ewah/ewah_bitmap.c |    8 ++++----
         ewah/ewok_rlw.h    |    6 +++---
         ppc/sha1.c         |    2 +-
         wrapper.c          |    2 +-
         8 files changed, 15 insertions(+), 15 deletions(-)



  reply	other threads:[~2021-11-17  9:19 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-15 23:19 [PATCH] mergesort: avoid left shift overflow René Scharfe
2021-11-16 14:08 ` Johannes Schindelin
2021-11-17  9:19   ` Junio C Hamano [this message]
2021-11-17 23:31     ` Johannes Schindelin
2021-11-18 23:27 ` Philip Oakley
2021-11-19 16:51   ` Johannes Schindelin
2021-11-19 17:15     ` René Scharfe
2021-11-19 17:27     ` Philip Oakley
2021-11-19 19:28       ` Junio C Hamano
2021-11-19 21:16         ` Philip Oakley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqr1bf2l83.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.