Hi Lennart,

Thanks for your comment.

It was my intention as I thought it would be better:
1) the caller to `handle_log_record()` no need to remember initializing
parameter.
Lately, `handle_log_record()` may be called by saLogWriteLog(). 
It will be problem if the caller forget to initialize the output data.

2) The caller should not know what `handle_log_record` doing with the out
parameter.


Regards, Vu.


>-----Original Message-----
>From: Lennart Lund [mailto:[email protected]]
>Sent: Tuesday, April 19, 2016 3:07 PM
>To: Vu Minh Nguyen; [email protected]
>Cc: [email protected]; Lennart Lund
>Subject: RE: [PATCH 1 of 1] log: fix incorrect time output [#1764]
>
>Hi Vu
>
>Ack with comment.
>
>I think it is a bit strange to give an unitialized parameter (logTimeStamp)
as
>input parameter to a function. I think it is better if logTimeStamp was
given a
>value before calling the function and not inside the function.
>
>Thanks
>Lennart
>
>> -----Original Message-----
>> From: Vu Minh Nguyen [mailto:[email protected]]
>> Sent: den 19 april 2016 05:12
>> To: [email protected]; Lennart Lund
>> Cc: [email protected]
>> Subject: [PATCH 1 of 1] log: fix incorrect time output [#1764]
>>
>>  osaf/libs/agents/saf/lga/lga_api.c |  11 ++++++-----
>>  1 files changed, 6 insertions(+), 5 deletions(-)
>>
>>
>> If timestamp (`logTimeStamp`) is not provided log client, log agent
>> will get the current time when getting the write log record request
>> and store the address, pointing to time holder variable, to
`logTimeStamp`.
>>
>> But the time holder variable was an local variable in function
>> `handle_log_record`,
>> then when exiting the function `handle_log_record`, the time holder
>> variable's content will be deleted.
>> So, the original time data is unknown after the calling.
>>
>> This patch fix the problem by not using the local variable to hold the
time
>> data.
>>
>> diff --git a/osaf/libs/agents/saf/lga/lga_api.c
>> b/osaf/libs/agents/saf/lga/lga_api.c
>> --- a/osaf/libs/agents/saf/lga/lga_api.c
>> +++ b/osaf/libs/agents/saf/lga/lga_api.c
>> @@ -957,10 +957,10 @@ SaAisErrorT saLogStreamOpenAsync_2(SaLog
>>   */
>>  static SaAisErrorT handle_log_record(const SaLogRecordT *logRecord,
>>      SaNameT *logSvcUsrName,
>> -    lgsv_write_log_async_req_t *write_param)
>> +    lgsv_write_log_async_req_t *write_param,
>> +    SaTimeT *const logTimeStamp)
>>  {
>>      SaAisErrorT ais_rc = SA_AIS_OK;
>> -    SaTimeT logTimeStamp;
>>
>>      TRACE_ENTER();
>>
>> @@ -999,8 +999,8 @@ static SaAisErrorT handle_log_record(con
>>
>>      /* Set timeStamp data if not provided by application user */
>>      if (logRecord->logTimeStamp == SA_TIME_UNKNOWN) {
>> -            logTimeStamp = setLogTime();
>> -            write_param->logTimeStamp = &logTimeStamp;
>> +            *logTimeStamp = setLogTime();
>> +            write_param->logTimeStamp = logTimeStamp;
>>      } else {
>>              write_param->logTimeStamp = (SaTimeT
>> *)&logRecord->logTimeStamp;
>>      }
>> @@ -1119,7 +1119,8 @@ SaAisErrorT saLogWriteLogAsync(SaLogStre
>>      /* Validate the log record and if generic header add
>>       * logSvcUsrName from environment variable
>> SA_AMF_COMPONENT_NAME
>>       */
>> -    ais_rc = handle_log_record(logRecord, &logSvcUsrName,
>> write_param);
>> +    SaTimeT logTimeStamp;
>> +    ais_rc = handle_log_record(logRecord, &logSvcUsrName,
>> write_param, &logTimeStamp);
>>      if (ais_rc != SA_AIS_OK) {
>>              TRACE("%s: Validate Log record Fail",
>> __FUNCTION__);
>>              goto done;


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to