Re: Minor issues in .pgpass

On Tue, Mar 3, 2020 at 5:38 PM Hamid Akhtar <hamid(dot)akhtar(at)gmail(dot)com> wrote:

>
>
> On Mon, Mar 2, 2020 at 6:07 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
> wrote:
>
>>
>>
>> On 2020/02/29 0:46, Hamid Akhtar wrote:
>> > The following review has been posted through the commitfest application:
>> > make installcheck-world: not tested
>> > Implements feature: not tested
>> > Spec compliant: not tested
>> > Documentation: not tested
>> >
>> > First of all, this seems like fixing a valid issue, albeit, the
>> probability of somebody messing is low, but it is still better to fix this
>> problem.
>> >
>> > I've not tested the patch in any detail, however, there are a couple of
>> comments I have before I proceed on with detailed testing.
>>
>> Thanks for the review and comments!
>>
>> > 1. pgindent is showing a few issues with formatting. Please have a look
>> and resolve those.
>>
>> Yes.
>>
>> > 2. I think you can potentially use "len" variable instead of
>> introducing "buflen" and "tmplen" variables.
>>
>> Basically I don't want to use the same variable for several purposes
>> because which would decrease the code readability.
>>
>
That is fine.

>
>> > Also, I would choose a more appropriate name for "tmp" variable.
>>
>> Yeah, so what about "rest" as the variable name?
>>
>
May be something like "excess_buf" or any other one that describes that
these bytes are to be discarded.

>
>> > I believe if you move the following lines before the conditional
>> statement and simply and change the if statement to "if (len >= sizeof(buf)
>> - 1)", it will serve the purpose.
>>
>> ISTM that this doesn't work correctly when the "buf" contains
>> trailing carriage returns but not newlines (i.e., this line is too long
>> so the "buf" doesn't include newline). In this case, pg_strip_crlf()
>> shorten the "buf" and then its return value "len" should become
>> less than sizeof(buf). So the following condition always becomes
>> false unexpectedly in that case even though there is still rest of
>> the line to eat.
>>
>
> Per code comments for pg_strip_crlf:
> "pg_strip_crlf -- Remove any trailing newline and carriage return"
>
> If the buf read contains a newline or a carriage return at the end, then
> clearly the line
> is not exceeding the sizeof(buf). If alternatively, it doesn't, then
> pg_strip_crlf will have
> no effect on string length and for any lines exceeding sizeof(buf), the
> following conditional
> statement becomes true.
>
>
>> > + if (len >= sizeof(buf) - 1)
>> > + {
>> > + char tmp[LINELEN];
>>
>> Regards,
>>
>> --
>> Fujii Masao
>> NTT DATA CORPORATION
>> Advanced Platform Technology Group
>> Research and Development Headquarters
>>
>
>
> --
> Highgo Software (Canada/China/Pakistan)
> URL : www.highgo.ca
> ADDR: 10318 WHALLEY BLVD, Surrey, BC
> CELL：+923335449950 EMAIL: mailto:hamid(dot)akhtar(at)highgo(dot)ca
> SKYPE: engineeredvirus
>

--
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL：+923335449950 EMAIL: mailto:hamid(dot)akhtar(at)highgo(dot)ca
SKYPE: engineeredvirus