Re: [PATCH v2] src/log.c: Include libgen.h for basename API

Hi all,

On 1/15/24 09:56, Marcel Holtmann wrote:
> Hi Khem,
> 
>> Use POSIX version of basename. This comes to front with latest musl
>> which dropped the declaration from string.h [1] it fails to build with
>> clang-17+ because it treats implicit function declaration as error.
>>
>> Fix it by applying the basename on a copy of string since posix version
>> may modify the input string.
>>
>> [1] https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7
>>
>> Signed-off-by: Khem Raj <[email protected]>
>> ---
>> v2: Fix formatting
>>
>> src/log.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/log.c b/src/log.c
>> index 554b046..2df3af7 100644
>> --- a/src/log.c
>> +++ b/src/log.c
>> @@ -24,6 +24,7 @@
>> #endif
>>
>> #include <stdio.h>
>> +#include <libgen.h>
>> #include <unistd.h>
>> #include <stdarg.h>
>> #include <stdlib.h>
>> @@ -196,6 +197,7 @@ int __connman_log_init(const char *program, const char *debug,
>> const char *program_name, const char *program_version)
>> {
>> static char path[PATH_MAX];
>> + char* tmp = strdup(program); >
> declaration are char *tmp;
> 

And perhaps use g_strdup() instead? coding-style.txt indirectly refers 
to use of g_ functions when available, as it is done with the rest of 
the codebase. Also, when small allocations fail the program should crash 
(M8).


>> int option = LOG_NDELAY | LOG_PID;
>>
>> program_exec = program;
>> @@ -212,8 +214,8 @@ int __connman_log_init(const char *program, const char *debug,
>> if (backtrace)
>> signal_setup(signal_handler);
>>
>> - openlog(basename(program), option, LOG_DAEMON);
>> -
> 
> Put the strdup here.
> 
> And you need to handle tmp == NULL since then it seems basename returns a constant “.”.
> 
>> + openlog(basename(tmp), option, LOG_DAEMON);
>> + free(tmp);
> 

I was just thinking maybe it would be better to handle the duplication 
of the program name, argv[0] in main.c (both connmand and vpnd), where 
it is given as initialization arg to __connman_log_init()?

Cheers,
  Jussi