Hi Canh,
Ack with minor comments inline.
Regards, Vu
> -----Original Message-----
> From: Canh Van Truong <canh.v.tru...@dektech.com.au>
> Sent: Monday, July 2, 2018 5:24 PM
> To: lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> <canh.v.tru...@dektech.com.au>
> Subject: [PATCH 1/1] osaf: update for saflog in case saLogWriteLogAsync
> with BAD_HANDLE [#2886]
>
> If saLogWriteLogAsync return BAD_HANDLE error, log client will never be
> re-initialized again. The patch reset variable "log_client_initialized =
false"
> in case BAD_HANDLE and the client will be re-initialized later.
> ---
> src/osaf/saflog/saflog.c | 75
++++++++++++++++--------------------------------
> 1 file changed, 25 insertions(+), 50 deletions(-)
>
> diff --git a/src/osaf/saflog/saflog.c b/src/osaf/saflog/saflog.c
> index 0c6ae5850..24312d162 100644
> --- a/src/osaf/saflog/saflog.c
> +++ b/src/osaf/saflog/saflog.c
> @@ -15,53 +15,48 @@
> * Author(s): Ericsson
> *
> */
> +#include "osaf/saflog/saflog.h"
>
> #include <stdio.h>
> -#include <syslog.h>
> #include <stdarg.h>
> +#include <stdbool.h>
> #include <unistd.h>
> -#include "osaf/saflog/saflog.h"
> #include <saAis.h>
> +#include "base/logtrace.h"
>
> -static int initialized;
> +static bool log_client_initialized;
[Vu] Should initialize this variable.
> static SaLogStreamHandleT logStreamHandle;
> static SaLogHandleT logHandle;
>
> void saflog_init(void)
> {
> - SaAisErrorT error;
> -
> - if (!initialized) {
> + if (log_client_initialized == false) {
[Vu] Checking the value with true might be better. Return if true, otherwise
do below.
Do that, we will not have one more level of indentation.
> SaVersionT logVersion = {'A', 2, 3};
> SaNameT stream_name;
> saAisNameLend(SA_LOG_STREAM_SYSTEM, &stream_name);
>
> - error = saLogInitialize(&logHandle, NULL, &logVersion);
> - if (error != SA_AIS_OK) {
> - syslog(LOG_INFO,
> - "saflogInit: saLogInitialize FAILED: %u",
error);
> + SaAisErrorT rc = saLogInitialize(&logHandle, NULL,
> &logVersion);
[Vu] Consider to add retry mechanism when calling SAF API.
> + if (rc != SA_AIS_OK) {
> + LOG_NO("saflogInit: saLogInitialize FAILED: %u",
rc);
> return;
> }
>
> - error = saLogStreamOpen_2(logHandle, &stream_name,
> NULL, 0,
> + rc = saLogStreamOpen_2(logHandle, &stream_name, NULL, 0,
> SA_TIME_ONE_SECOND,
> &logStreamHandle);
> - if (error != SA_AIS_OK) {
> - syslog(LOG_INFO,
> - "saflogInit: saLogStreamOpen_2 FAILED: %u",
> - error);
> + if (rc != SA_AIS_OK) {
> + LOG_NO("saflogInit: saLogStreamOpen_2 FAILED: %u",
> rc);
> if (saLogFinalize(logHandle) != SA_AIS_OK)
> - syslog(LOG_INFO,
> - "saflogInit: saLogFinalize FAILED:
%u",
> - error);
> + LOG_NO("saflogInit: saLogFinalize FAILED:
%u",
> + rc);
> return;
> }
> - initialized = 1;
> + log_client_initialized = true;
> }
> }
>
> void saflog(int priority, const SaNameT *logSvcUsrName, const char
> *format, ...)
> {
> - SaAisErrorT error;
> + SaAisErrorT rc;
> SaLogRecordT logRecord;
> SaLogBufferT logBuffer;
> va_list ap;
> @@ -74,34 +69,14 @@ void saflog(int priority, const SaNameT
> *logSvcUsrName, const char *format, ...)
> va_end(ap);
>
> if (logBuffer.logBufSize > SA_LOG_MAX_RECORD_SIZE) {
> - syslog(LOG_INFO,
> - "saflog write FAILED: log record size > %u max
limit",
> + LOG_NO("saflog write FAILED: log record size > %u max
limit",
> SA_LOG_MAX_RECORD_SIZE);
> return;
> }
>
> - if (!initialized) {
> - SaVersionT logVersion = {'A', 2, 3};
> - SaNameT stream_name;
> - saAisNameLend(SA_LOG_STREAM_SYSTEM, &stream_name);
> -
> - error = saLogInitialize(&logHandle, NULL, &logVersion);
> - if (error != SA_AIS_OK) {
> - syslog(LOG_INFO, "saLogInitialize FAILED: %u",
error);
> - goto done;
> - }
> -
> - error = saLogStreamOpen_2(logHandle, &stream_name,
> NULL, 0,
> - SA_TIME_ONE_SECOND,
> &logStreamHandle);
> - if (error != SA_AIS_OK) {
> - syslog(LOG_INFO, "saLogStreamOpen_2 FAILED: %u",
> error);
> - if (saLogFinalize(logHandle) != SA_AIS_OK)
> - syslog(LOG_INFO, "saLogFinalize FAILED: %u",
> - error);
> - goto done;
> - }
> - initialized = 1;
> - }
> + // Initialize log client and open system stream if they are NOT
> + saflog_init();
> + if (log_client_initialized == false) return;
>
> logRecord.logTimeStamp = SA_TIME_UNKNOWN;
> logRecord.logHdrType = SA_LOG_GENERIC_HEADER;
> @@ -111,11 +86,11 @@ void saflog(int priority, const SaNameT
> *logSvcUsrName, const char *format, ...)
> logRecord.logBuffer = &logBuffer;
> logBuffer.logBuf = (SaUint8T *)str;
>
> - error = saLogWriteLogAsync(logStreamHandle, 0, 0, &logRecord);
> + rc = saLogWriteLogAsync(logStreamHandle, 0, 0, &logRecord);
[Vu] Consider to add retry mechanism when calling SAF API
>
> -done:
> - /* fallback to syslog at ANY error, syslog prio same as saflog
severity
> - */
> - if (error != SA_AIS_OK)
> - syslog(priority, "%s", str);
> + if (rc != SA_AIS_OK) {
> + LOG_NO("saflog write \"%s\" FAILED", str);
> + if (rc == SA_AIS_ERR_BAD_HANDLE)
[Vu] Finalize log handle here.
> + log_client_initialized = false;
> + }
> }
> --
> 2.15.1
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
