Horton

Thread

Commits

  1. Remove redundant memset call following palloc0

  1. remove redundant memset() call

    Zhihong Yu <zyu@yugabyte.com> — 2022-10-13T17:55:08Z

    Hi,
I was looking at combo_init in contrib/pgcrypto/px.c .

There is a memset() call following palloc0() - the call is redundant.

Please see the patch for the proposed change.

Thanks

  2. Re: remove redundant memset() call

    Bruce Momjian <bruce@momjian.us> — 2022-10-13T19:10:16Z

    On Thu, Oct 13, 2022 at 10:55:08AM -0700, Zhihong Yu wrote:
> Hi,
> I was looking at combo_init in contrib/pgcrypto/px.c .
> 
> There is a memset() call following palloc0() - the call is redundant.
> 
> Please see the patch for the proposed change.
> 
> Thanks

> diff --git a/contrib/pgcrypto/px.c b/contrib/pgcrypto/px.c
> index 3b098c6151..d35ccca777 100644
> --- a/contrib/pgcrypto/px.c
> +++ b/contrib/pgcrypto/px.c
> @@ -203,7 +203,6 @@ combo_init(PX_Combo *cx, const uint8 *key, unsigned klen,
>  	if (klen > ks)
>  		klen = ks;
>  	keybuf = palloc0(ks);
> -	memset(keybuf, 0, ks);
>  	memcpy(keybuf, key, klen);
>  
>  	err = px_cipher_init(c, keybuf, klen, ivbuf);

Uh, the memset() is ks length but the memcpy() is klen, and the above
test allows ks to be larger than klen.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Indecision is a decision.  Inaction is an action.  Mark Batterson

  3. Re: remove redundant memset() call

    Zhihong Yu <zyu@yugabyte.com> — 2022-10-13T19:12:35Z

    On Thu, Oct 13, 2022 at 12:10 PM Bruce Momjian <bruce@momjian.us> wrote:

> On Thu, Oct 13, 2022 at 10:55:08AM -0700, Zhihong Yu wrote:
> > Hi,
> > I was looking at combo_init in contrib/pgcrypto/px.c .
> >
> > There is a memset() call following palloc0() - the call is redundant.
> >
> > Please see the patch for the proposed change.
> >
> > Thanks
>
> > diff --git a/contrib/pgcrypto/px.c b/contrib/pgcrypto/px.c
> > index 3b098c6151..d35ccca777 100644
> > --- a/contrib/pgcrypto/px.c
> > +++ b/contrib/pgcrypto/px.c
> > @@ -203,7 +203,6 @@ combo_init(PX_Combo *cx, const uint8 *key, unsigned
> klen,
> >       if (klen > ks)
> >               klen = ks;
> >       keybuf = palloc0(ks);
> > -     memset(keybuf, 0, ks);
> >       memcpy(keybuf, key, klen);
> >
> >       err = px_cipher_init(c, keybuf, klen, ivbuf);
>
> Uh, the memset() is ks length but the memcpy() is klen, and the above
> test allows ks to be larger than klen.
>
> --
>   Bruce Momjian  <bruce@momjian.us>        https://momjian.us
>   EDB                                      https://enterprisedb.com
>
>   Indecision is a decision.  Inaction is an action.  Mark Batterson
>
> Hi,
the memory has been zero'ed out by palloc0().

memcpy is not relevant w.r.t. resetting memory.

Cheers

  4. Re: remove redundant memset() call

    Bruce Momjian <bruce@momjian.us> — 2022-10-13T19:15:13Z

    On Thu, Oct 13, 2022 at 12:12:35PM -0700, Zhihong Yu wrote:
> On Thu, Oct 13, 2022 at 12:10 PM Bruce Momjian <bruce@momjian.us> wrote:
> 
>     On Thu, Oct 13, 2022 at 10:55:08AM -0700, Zhihong Yu wrote:
>     > Hi,
>     > I was looking at combo_init in contrib/pgcrypto/px.c .
>     >
>     > There is a memset() call following palloc0() - the call is redundant.
>     >
>     > Please see the patch for the proposed change.
>     >
>     > Thanks
> 
>     > diff --git a/contrib/pgcrypto/px.c b/contrib/pgcrypto/px.c
>     > index 3b098c6151..d35ccca777 100644
>     > --- a/contrib/pgcrypto/px.c
>     > +++ b/contrib/pgcrypto/px.c
>     > @@ -203,7 +203,6 @@ combo_init(PX_Combo *cx, const uint8 *key, unsigned
>     klen,
>     >       if (klen > ks)
>     >               klen = ks;
>     >       keybuf = palloc0(ks);
>     > -     memset(keybuf, 0, ks);
>     >       memcpy(keybuf, key, klen);
>     > 
>     >       err = px_cipher_init(c, keybuf, klen, ivbuf);
> 
>     Uh, the memset() is ks length but the memcpy() is klen, and the above
>     test allows ks to be larger than klen.
> 
>     --
>       Bruce Momjian  <bruce@momjian.us>        https://momjian.us
>       EDB                                      https://enterprisedb.com
> 
>       Indecision is a decision.  Inaction is an action.  Mark Batterson
> 
> 
> Hi,
> the memory has been zero'ed out by palloc0().
> 
> memcpy is not relevant w.r.t. resetting memory.

Ah, good point.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Indecision is a decision.  Inaction is an action.  Mark Batterson

  5. Re: remove redundant memset() call

    Nathan Bossart <nathandbossart@gmail.com> — 2022-10-13T19:18:41Z

    On Thu, Oct 13, 2022 at 03:15:13PM -0400, Bruce Momjian wrote:
> On Thu, Oct 13, 2022 at 12:12:35PM -0700, Zhihong Yu wrote:
>> the memory has been zero'ed out by palloc0().
>> 
>> memcpy is not relevant w.r.t. resetting memory.
> 
> Ah, good point.

Yeah, it looks like this was missed in ca7f8e2.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  6. Re: remove redundant memset() call

    Daniel Gustafsson <daniel@yesql.se> — 2022-10-13T19:40:42Z

    > On 13 Oct 2022, at 21:18, Nathan Bossart <nathandbossart@gmail.com> wrote:
> 
> On Thu, Oct 13, 2022 at 03:15:13PM -0400, Bruce Momjian wrote:
>> On Thu, Oct 13, 2022 at 12:12:35PM -0700, Zhihong Yu wrote:
>>> the memory has been zero'ed out by palloc0().
>>> 
>>> memcpy is not relevant w.r.t. resetting memory.
>> 
>> Ah, good point.
> 
> Yeah, it looks like this was missed in ca7f8e2.

Agreed, it looks like I missed that one, I can't see any reason to keep it.  Do
you want me to take care of it Bruce, and clean up after myself, or are you
already on it?

--
Daniel Gustafsson		https://vmware.com/

  7. Re: remove redundant memset() call

    Bruce Momjian <bruce@momjian.us> — 2022-10-13T19:59:43Z

    On Thu, Oct 13, 2022 at 09:40:42PM +0200, Daniel Gustafsson wrote:
> > On 13 Oct 2022, at 21:18, Nathan Bossart <nathandbossart@gmail.com> wrote:
> > 
> > On Thu, Oct 13, 2022 at 03:15:13PM -0400, Bruce Momjian wrote:
> >> On Thu, Oct 13, 2022 at 12:12:35PM -0700, Zhihong Yu wrote:
> >>> the memory has been zero'ed out by palloc0().
> >>> 
> >>> memcpy is not relevant w.r.t. resetting memory.
> >> 
> >> Ah, good point.
> > 
> > Yeah, it looks like this was missed in ca7f8e2.
> 
> Agreed, it looks like I missed that one, I can't see any reason to keep it.  Do
> you want me to take care of it Bruce, and clean up after myself, or are you
> already on it?

You can do it, thanks.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Indecision is a decision.  Inaction is an action.  Mark Batterson

  8. Re: remove redundant memset() call

    Daniel Gustafsson <daniel@yesql.se> — 2022-10-13T21:57:47Z

    > On 13 Oct 2022, at 21:59, Bruce Momjian <bruce@momjian.us> wrote:
> 
> On Thu, Oct 13, 2022 at 09:40:42PM +0200, Daniel Gustafsson wrote:
>>> On 13 Oct 2022, at 21:18, Nathan Bossart <nathandbossart@gmail.com> wrote:
>>> 
>>> On Thu, Oct 13, 2022 at 03:15:13PM -0400, Bruce Momjian wrote:
>>>> On Thu, Oct 13, 2022 at 12:12:35PM -0700, Zhihong Yu wrote:
>>>>> the memory has been zero'ed out by palloc0().
>>>>> 
>>>>> memcpy is not relevant w.r.t. resetting memory.
>>>> 
>>>> Ah, good point.
>>> 
>>> Yeah, it looks like this was missed in ca7f8e2.
>> 
>> Agreed, it looks like I missed that one, I can't see any reason to keep it. Do
>> you want me to take care of it Bruce, and clean up after myself, or are you
>> already on it?
> 
> You can do it, thanks.

Done now.

--
Daniel Gustafsson		https://vmware.com/