Re: [PATCH] fixing -logfile when used with -displayfd
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
>>> ---
>>> 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);
>>>
>>
>
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] fixing -logfile when used with -displayfd
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?
-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
---
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);
--
-Alan Coopersmith- alan.coopersm...@oracle.com
Oracle Solaris Engineering - https://blogs.oracle.com/alanc
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] fixing -logfile when used with -displayfd
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
> ---
> 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);
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH] fixing -logfile when used with -displayfd
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
---
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);
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
