GNOME Bugzilla – Bug 668039
Lock and logout delays capped to 480s despite what control center pretends
Last modified: 2012-01-17 14:21:04 UTC
For several releases I noticed the lock delay I wanted (one hour) wasn't honored. Now that I investigated this, I discovered something incredible in gs-prefs.c: static void _gs_prefs_set_lock_timeout (GSPrefs *prefs, guint value) { /* pick a reasonable large number for the upper bound */ if (value > 480) value = 480; prefs->lock_timeout = value * 1000; } I think this dates back to the times when value was in minutes. Now that it's in seconds, this limits the timeout to 8 minutes, when the control center allows to choose "1 hour". Commit a50f7dc9, which changed value * 60000 to value * 1000, forgot to multiply the max by 60 too. This affects the logout delay too. I don't see the interest of limiting the delay, so I'd say change it to G_MAXUINT. Attached is a patch to do that.
Created attachment 205393 [details] [review] Described patch
Review of attachment 205393 [details] [review]: hah funny bug :-) Thanks for figuring this out. Patch looks mostly good. Minor nits below: You're actually doing two separate things here 1) Fixing the "off by a factor of 60" problem and 2) changing the upper limit. Both are probably fine, though, I'd rather you did it in two commits. First, fix the bug so it works how it was originally intended to work, second bump the upper limit. ::: src/gs-prefs.c @@ +131,3 @@ + /* prevent overflow when converting to milliseconds */ + if (value > G_MAXUINT/1000) + value = G_MAXUINT/1000; need spaces around operators and braces around one line if statements (i realize it was already wrong to begin with) @@ +219,3 @@ + /* prevent overflow when converting to milliseconds */ + if (value > G_MAXUINT/1000) + value = G_MAXUINT/1000; same here.
Thanks for the review. I'm not doing two separate thing at all: there's no "off by a factor of 60" problem at all (this was changed in commit a50f7dc9). I'm just changing the too-low-and-wrong limit of 480 to the max we can use, that's all.
Oh, I see now that you meant I was removing the limit to 480 minutes while I should make it work again first. I can do that, of course, but it would really look silly unless we find a justification for this value. And if we do find it, we can as well keep using it... ;-)
You could have one commit like this: prefs: scale max lock timeout to proper units gnome-screensaver tries to cap the maximum amount of idle time before screen lock to 8 hours. commit a50f7dc9 changed the internal representation of this timeout from minutes to seconds, but the cap wasn't appropriately scaled. This commit raises fixes the cap to again be 8 hours. and then a follow up commit like this: prefs: drop max lock timeout Previously, gnome-screensaver would enforce a maximum lock timeout to prevent situations where the screen could have locking enabled, but never lock in practice. The screen control panel enforces a different maximum lock timeout though its UI. There's no reason to have two out of sync maximums, so this commit removes the gnome-screensaver one. I think it's cleaner to do it in two commits, but if you would rather do it as one commit then make sure you mention both changes in the commit message.
Sure. Pushed as f7ec7a and b782e17.