[PATCH] fixing -logfile when used with -displayfd

[Antoine Martin]

On 28/08/17 13:02, Alan Coopersmith wrote:
> I admit not looking at this code in a year and a half and barely
> remembering
> what it does, but that seems like the wrong place to add the check -
> wouldn't
> it make more sense to change
>     if (displayfd != -1) {
> to
>     if ((displayfd != -1) && strstr(saved_log_fname, "%s")) {
> in LogInit, so that the saved_log_* variables never get set in the first
> place?
You're right, this is a much more logical place to patch.
Only you need to call strstr on "fname" here, like so:


@@ -247,7 +247,7 @@ LogInit(const char *fname, const char *backup)
     char *logFileName = NULL;

     if (fname && *fname) {
-        if (displayfd != -1) {
+        if (displayfd != -1 && strstr(fname, "%s")) {
             /* Display isn't set yet, so we can't use it in filenames
yet. */
             char pidstring[32];
             snprintf(pidstring, sizeof(pidstring), "pid-%ld",


Antoine


> 
>     -alan-
> 
> On 08/27/17 10:21 PM, Antoine Martin wrote:
>> Bump. This is a cosmetic bug, but it trips up many people when they
>> cannot find the log file that they had requested.
>>
>> This should be applied to older branches too since this particular bug
>> was also applied to 1.18.1 (a fairly big change too, especially compared
>> to this one liner fix for it)
>>
>> Cheers
>> Antoine
>>
>>
>> On 05/08/17 17:09, Antoine Martin wrote:
>>> Hi,
>>>
>>> Trivial way to reproduce the bug:
>>> Xorg -logfile /tmp/mylog -config /etc/xpra/xorg.conf -displayfd 2
>>>
>>> The server then moans:
>>> Failed to rename log file "/tmp/mylog" to "/tmp/mylog": No such file or
>>> directory
>>>
>>> And the log file is created, but immediately renamed to "/tmp/mylog.old"
>>>
>>> This is caused by the changes to the log file handling introduced by
>>> this commit:
>>> https://cgit.freedesktop.org/xorg/xserver/commit/?id=edcb6426f20c3be5dd5f50b76a686754aef2f64e
>>>
>>> And below is the trivial fix for it.
>>>
>>> ---
>>> Don't try to add the pidstring to the log filename if it doesn't contain
>>> the "%s" placeholder for it.
>>>
>>> Signed-off-by: Antoine Martin <antoine at nagafix.co.uk>
>>> ---
>>> diff --git a/os/log.c b/os/log.c
>>> index 91e55a532..a3b28ccb4 100644
>>> --- a/os/log.c
>>> +++ b/os/log.c
>>> @@ -296,7 +296,7 @@ LogInit(const char *fname, const char *backup)
>>>   void
>>>   LogSetDisplay(void)
>>>   {
>>> -    if (saved_log_fname) {
>>> +    if (saved_log_fname && strstr(saved_log_fname, "%s")) {
>>>           char *logFileName;
>>>
>>>           logFileName = LogFilePrep(saved_log_fname, saved_log_backup,
>>> display);
>>>
>>
> 
>