Ben --- I'd very much like a change to this patch.  Introducing the
'cmd' member of cmd_parms constitutes a change to the API; I really
think these should be discussed on the list before being committed.
This one, in any case, is unnecessary.  The cmd_data of the callback
structure already exists precisely to allow a command handler to
behave differently depending on which command table entry caused it to
be invoked; if the relevant command table entries looked like

  { "RLimitCPU", no_set_limit, "RLimitCPU", ... },
  { "RLimitNPROC", no_set_limit, "RLimitNPROC", ... },

the name under which no_set_limit was invoked would be in cmd->info.

As long as I'm looking the code more over with a tighter eye than
formally (sigh...), I just noticed the use of RAW_ARGS; this is
something we may want to change before final release --- the problem
with it is that while TAKE1, TAKE2, and the like provide some sort of
guidance to an external agent which is trying to parse the config file
(perhaps through a mod_info-style intermediary), RAW_ARGS is
essentially useless.

I understand that you aren't personally responsible for this, but
still, in the long run, we ought to change it.  (I've been thinking
that maybe adding a TAKE2OPT-style command-handling directive, which
would be like TAKE2 except that it allowed the args to be missing,
would be a good idea).

One last gripe --- the command handlers seem to be logging errors in
cases where they really ought to be returning a syntax error string
--- i.e., in cases where the args are just syntactically invalid.
This really ought to change...

rst

Reply via email to