Re: Concurrent psql patch - Mailing list pgsql-patches

From Pavan Deolasee
Subject Re: Concurrent psql patch
Date
Msg-id 2e78013d0705180459o27d6c412xeb8c14c1bfcb2f53@mail.gmail.com
Whole thread Raw
In response to Re: Concurrent psql patch  (Bruce Momjian <bruce@momjian.us>)
Responses Re: Concurrent psql patch
List pgsql-patches

Hi Greg,

I looked at the patch briefly. I couldn't spot any issues and it looks good
to me. I've just couple of comments here.

The mvcc regression test files are missing in the patch.

--- 1179,1189 ----
                              dbname, user, password);

        /* We can immediately discard the password -- no longer needed */
!       if (password)
!       {
!           memset(password, '\0', strlen(password));
            free(password);
+       }


Any reason why we do this ? "password" is anyways freed.  I think you
might have left it behind after some debugging exercise.


--- 25,37 ----
  #include "mb/pg_wchar.h"
  #include "mbprint.h"

+ #if 0
+ #include "libpq-int.h" /* For PG_ASYNC */
+ #endif
+

This looks redundant..

Apart from that I really like consistent coding style. For example, to me,
"for (i = 0; i < 10; i++)" looks much better than "for (i=0;i<10; i++)"

This is not comment on your patch  and neither I am saying
we should follow a specific coding style (though I wish we could have done
so) because we have already so many different styles. So its best to
stick to the coding style already followed in that particular file. But few
simple rules like having a single space around operators like '<', '+', '='
etc really makes the code more readable. Other examples are using
parenthesis in a right manner to improve code readability.

flag = (pointer == NULL); is more readable than
flag = pointer == NULL;

Thanks,
Pavan

--
Pavan Deolasee
EnterpriseDB     http://www.enterprisedb.com

pgsql-patches by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: Updated bitmap index patch
Next
From: Heikki Linnakangas
Date:
Subject: Re: Maintaining cluster order on insert