Re: pg_ctl.c

Andrew Dunstan wrote:

> Gaetano Mendola wrote:
>
>> Andrew Dunstan wrote:
>>
>>> Gaetano Mendola wrote:
>>>
>>>> Bruce Momjian wrote:
>>>>
>>>> however below the result of my quich review:
>>>>
>>>> 1) exit(1) => exit(EXIT_FAILURE)
>>>
>>>
>>>
>>>
>>>
>>> If we used a number of different error codes I might agree. But it
>>> seems pointless here, and the style is widely used in our code base
>>> (I just counted 201 other occurrrences, not including cases of
>>> exit(0) ).
>>
>>
>>
>> This doesn't mean that we don't have to.
>
>
>
> We should be consistent. If you want to prepare a global patch that
> replaces every instance of exit(n) with exit(SOME_CONSTANT) then be my
> guest.

I'd like to do it, partecipate more on postgres coding but in this
period I really don't have time, this doesn't mean that we have to
wrote exit(1) instead of exit(EXIT_FAILURE) and change these
exit in the future. BTW the next patch will write again: exit(1).

>>>> 2) xstrdup protected by duplicate NULL string
>>>
>>>
>>> I don't object, but it is redundant - in every case where it is
>>> called the argument is demonstrably not NULL.
>>
>>
>>
>> Now it's true, and in the future ? Bruce was arguing about that check
>> that if the string is null the program simply will exit crashing!
>> I really appreciate the quality software of Postgres but some time
>> I don't understand why test "NULL" pointer is an overkill for you.
>> I mean xstrdup is supposed to be the strdup safe version, and without
>> that control is not safe, why don't use directly the strdup then ?
>> If there is no memory available before postgresql start go figure after!
>>
>
> I am not arguing that we should crash. I am arguing that we will not
> crash in this case, and the test is therefore redundant.
>
> I already said I don't object to the change, if that's the consensus,
> just that it is unnecessary. BTW, this code was lifted directly from
> initdb.c.
>
> I'd far rather you found real bugs than the ghosts of imaginary bugs,
> though.

I was not looking for bugs or ghost, I repeat the test may be redundant
now with the current version of pg_ctl.c, do you write free function
trusting in the caller ( as Bruce do ) or watching were and how it's called?

I write free function without trust the caller and without see where and
how is called, different point of view.

Regards
Gaetano Mendola