vncsession: Move existing log to log.old if present

[jcpunk]

This is my attempt to resolve #1145

[CendioOssman]

Looks good! Thanks!

I just want to do a test run and then this can be merged.

[CendioOssman]

It seems gcc is unhappy with the changes, unfortunately. :/

I'm unable to reproduce the issue here with gcc 13. I wonder if gcc 11 is more aggressive with this warning.

[jcpunk]

I'll rework to completely copy this string generation.

[CendioOssman]

I'm afraid the adjusted code does not work:

Aug 06 10:55:30 ubuntu2404 vncsession[3089]: Failure renaming log file "/home/tltest/.local/state/tigervnc/ubuntu2404:2.log" to "/ubuntu2404:2.log.old": Permission denied

I would suggest splitting out the base directory to a separate variable. That should make things more clear, and avoid such mistakes.

[jcpunk]

I'm out of the office right now but took a quick stab at it... might be garbage but wanted to try...

[CendioOssman]

I figured out why the errors are only shown sporadically. Apparently, they go away if you build with any optimisation flags. Weird.

Still, it looks like some error handling is needed here. Probably best to add it to all the snprintf() calls there.

[jcpunk]

I'll confess I don't understand what is required.

[CendioOssman]

Something like this:

diff --git a/unix/vncserver/vncsession.c b/unix/vncserver/vncsession.c
index 98a0432aa..8fbcb7703 100644
--- a/unix/vncserver/vncsession.c
+++ b/unix/vncserver/vncsession.c
@@ -408,10 +408,19 @@ redir_stdio(const char *homedir, const char *display, char **envp)
     close(fd);
 
     xdgstate = getenvp("XDG_STATE_HOME", envp);
-    if (xdgstate != NULL && xdgstate[0] == '/')
-        snprintf(logfile, sizeof(logfile), "%s/tigervnc", xdgstate);
-    else
-        snprintf(logfile, sizeof(logfile), "%s/.local/state/tigervnc", homedir);
+    if (xdgstate != NULL && xdgstate[0] == '/') {
+        size_t len = snprintf(logfile, sizeof(logfile), "%s/tigervnc", xdgstate);
+        if (len >= sizeof(logfile)) {
+            syslog(LOG_CRIT, "Log path too long");
+            _exit(EX_OSERR);
+        }
+    } else {
+        size_t len = snprintf(logfile, sizeof(logfile), "%s/.local/state/tigervnc", homedir);
+        if (len >= sizeof(logfile)) {
+            syslog(LOG_CRIT, "Log path too long");
+            _exit(EX_OSERR);
+        }
+    }
 
     snprintf(legacy, sizeof(legacy), "%s/.vnc", homedir);
     if (stat(logfile, &st) != 0 && stat(legacy, &st) == 0) {

[jcpunk]

Hopefully the current patch is closer to what you expect.

[CendioOssman]

Nice! Everything seems to work nicely when I do test run on a Ubuntu 24.04 machine here.

Thanks for getting this through to the end!