On Thu, Dec 12, 2013 at 4:43 PM, MauMau <maumau307@gmail.com> wrote:
> Hi, Amit san,
>
> From: "Amit Kapila" <amit.kapila16@gmail.com>
>>>
>>> [elog.c]
>>> Writing the default value in this file was redundant, because
>>> event_source
>>> cannot be NULL. So change
>>>
>> I think this change might not be safe as write_eventlog() gets called
>> from write_stderr() which might get called
>> before reading postgresql.conf, so the code as exists in PG might be
>> to handle that case or some other similar
>> situation where event_source is still not set. Have you confirmed in
>> all ways that it is never the case that
>> event_source is not set when the control reaches this function.
>
>
> Yes, you are right. I considered this again after replying to your previous
> mail, and found out write_stderr() calls write_eventlog(). In that case,
> event_source is still NULL before GUC initialization.
Few minor things:
1.
evtHandle = RegisterEventSource(NULL,
*event_source? event_source: DEFAULT_EVENT_SOURCE);
In this code, you are trying to access the value (*event_source) and
incase it is not initialised,
it will not contain the value and could cause problem, why not
directly check 'event_source'?
2. minor coding style issue
pg_ctl.c
evtHandle = RegisterEventSource(NULL,
*event_source? event_source: DEFAULT_EVENT_SOURCE);
elog.c
! evtHandle = RegisterEventSource(NULL,
! event_source ? event_source : DEFAULT_EVENT_SOURCE);
In both above usages, it is better that arguments in second line should start
inline with previous lines first argument. You can refer other places,
for ex. refer call to ReportEvent in pg_ctl.c just below
RegisterEventSource call.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com