[Pm-utils] [RFC] [patch review] Hook independence and security fixups, part 1

[Dan Nicholson]

On Feb 2, 2008 3:22 PM, Victor Lowther <victor.lowther at gmail.com> wrote:
> On Feb 2, 2008 4:32 PM, Victor Lowther <victor.lowther at gmail.com> wrote:
> > Hmmm... how about something along these lines:
> >
> > @PM-UTILS-LIBDIR@/methods.d/, which is populated by files named
> >   suspend_method-suspend_type (kernel-suspend, kernel-hibernate,
> > tuxonice-suspend-hybrid, etc.)
> >   These files accept a single argument, which (right now) can be
> > either "inhibited" or "", and output either of "resume" or "thaw" on
> > stdout, depending on whether the system is resuming from suspend or
> > hibernate.
> >
> > All the do_suspend_type functions then collapse into a single
> > do_sleep() function, which is passed either "hibernate", "suspend", or
> > "suspend-hybrid".  The do_sleep method then calls
> > @PM-UTILS-LIBDIR@/method.d/${SUSPEND_METHOD}-$1 with the appropriate
> > value of inhibit
> >
> > That way, there is no patching of case statements needed to add a new
> > suspend method -- just drop an appropriatly-named script in the
> > method.d directory, and let it handle doing The Right Thing.
>
> Test patch attached.  It may not actually run, and there are a few
> niceties to add, but it is a first stab at things.  Should apply
> cleanly to current git head, and is also available as
> vlowther-pluggable-sleep-methods in my git repo.

I don't see many flaws in your patch, except that you have methoddir =
.../config.d and you removed the zzz hook, which doesn't have to do
with this change. My personal feeling is that the methods.d stuff is
adding more complexity than the gain it provides. There's definitely a
benefit to being able to drop in a new file and change a config
variable to use a new suspend backend. However, passing a command to
run_hooks that it will expand is not something I find particularly
attractive.

The alternative that I suggested before of just having a case
statement with the accepted methods is conceptually much simpler,
especially for reading and understanding the code. In this case,
adding tuxonice support means adding one option to a case statement
and one or two functions. I would also hope that the number of suspend
methods is decreasing to the point where having a modular backend
doesn't provide much gain.

Just my thoughts.