[libcamera-devel] [PATCH] libcamera: keep using syslog logging target also for isolated IPA modules

[Matthias Fend]

Hi Jacopo,

Am 20.02.2023 um 11:01 schrieb Jacopo Mondi:
> Hello Matthias
> 
> On Mon, Feb 20, 2023 at 10:13:06AM +0100, Matthias Fend wrote:
>> Hi Jacopo,
>>
>> Am 20.02.2023 um 09:50 schrieb Jacopo Mondi:
>>> Hello Matthias
>>>
>>> On Mon, Feb 20, 2023 at 08:43:52AM +0100, Matthias Fend via libcamera-devel wrote:
>>>> Currently it is not possible to display debug output from an isolated IPA
>>>> module. The standard  descriptors are all closed and any specified log
>>>> file is explicitly deactivated for the IPA module. Since libcamera and the
>>>> isolated IPA modul are separate processes, they cannot write to the same
>>>> file. However, if syslog is used, then this would be possible.
>>>>
>>>> If syslog is specified as a log file, then this is left as it is for the
>>>> isolated IPA module.
>>>>
>>>
>>> I'm certainly missing something, but doesn't logging to syslogd
>>> happens through the 'syslog()'[1] function call ?
>>
>> If LIBCAMERA_LOG_FILE is set to 'syslog', the logging target is set to
>> 'LoggingTargetSyslog', which instructs the libcamera logger to use syslog()
>> to output log messages. This is already implemented.
> 
> Ups, I completely missed that
> 
>>
>> However, this environment variable is explicitly deleted for an isolated IPA
>> module. This change only prevents this behavior if syslog is to be used.
>>
>> I don't see any other way to see the debug output of such a module - apart
>> from adapting the libcamera source code of course.
>>
> 
> Ok then, this now makes sense to me.
> 
> I don't see issues in allowing isolated processes interface to syslog,
> but let's wait for other's opinions.
> 
>> ~Matthias
>>
>>>
>>> [1] https://linux.die.net/man/3/syslog
>>>
>>>> Signed-off-by: Matthias Fend <matthias.fend at emfend.at>
>>>> ---
>>>>    src/libcamera/process.cpp | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
>>>> index 0e6b4e1d..86a382fb 100644
>>>> --- a/src/libcamera/process.cpp
>>>> +++ b/src/libcamera/process.cpp
>>>> @@ -263,7 +263,9 @@ int Process::start(const std::string &path,
>>>>
>>>>    		closeAllFdsExcept(fds);
>>>>
>>>> -		unsetenv("LIBCAMERA_LOG_FILE");
>>>> +		const char *file = utils::secure_getenv("LIBCAMERA_LOG_FILE");
>>>> +		if (file && strcmp(file, "syslog"))
>>>> +			unsetenv("LIBCAMERA_LOG_FILE");
> 
> Indentation seems off though

Unfortunately, I do not see the problem. Can you please tell me more 
precisely what does not fit?

Thanks
  ~Matthias

> 
>>>>
>>>>    		const char **argv = new const char *[args.size() + 2];
>>>>    		unsigned int len = args.size();
>>>> --
>>>> 2.25.1
>>>>