Re: Remove dependence on integer wrapping - Mailing list pgsql-hackers

From Nathan Bossart
Subject Re: Remove dependence on integer wrapping
Date
Msg-id Zmh5wxds3DlyWAZy@nathan
Whole thread Raw
In response to Re: Remove dependence on integer wrapping  (Joseph Koshakow <koshy44@gmail.com>)
Responses Re: Remove dependence on integer wrapping
List pgsql-hackers
On Tue, Jun 11, 2024 at 09:31:39AM -0400, Joseph Koshakow wrote:
> tmp is an uint16 here, it seems like you might have read it as an
> int16? We would need some helper method like
> 
>         static inline bool
>         pg_neg_u16_overflow(uint16 a, int16 *result);
> 
> and then we could replace that whole chunk with something like
> 
>         if (unlikely(pg_neg_u16_overflow(tmp, &result)))
>                 goto out_of_range;
>         else
>                 return result;
> 
> 
> that pattern shows up a lot in this file, but I was worried that it
> wasn't useful as a general purpose function. Happy to add it
> though if you still feel otherwise.

I personally find that much easier to read.  Since the existing open-coded
overflow check is apparently insufficient, I think there's a reasonably
strong case for centralizing this sort of thing so that we don't continue
to make the same mistakes.

>> Ah, I see.  Joe's patch does that in one place.  It's probably worth doing
>> that in the other places this "just-barefly overflow" comment appears
>> IMHO.
> 
> The only other place I found this comment was in
> `make_timestamp_internal`. I've updated that function and added some
> tests. I also manually verified that the behavior matches before and
> after this patch.

tm2timestamp() in src/interfaces/ecpg/pgtypeslib/timestamp.c has the same
comment.  The code there looks very similar to the code for tm2timestamp()
in the other timestamp.c...

-- 
nathan



pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Keeping track of buildfarm animals' personality
Next
From: Ranier Vilela
Date:
Subject: Re: Improve the granularity of PQsocketPoll's timeout parameter?