[PATCH] Fix bug 13465

Christof Schmitt cs at samba.org
Wed Aug 15 21:40:48 UTC 2018


On Thu, Aug 09, 2018 at 06:05:19PM +0200, Volker Lendecke via samba-technical wrote:
> On Fri, Aug 03, 2018 at 04:39:18PM -0700, Christof Schmitt via samba-technical wrote:
> > Here is an updated patch set based on the above observations. A cluster
> > requires root access and an error message is printed for that case:
> 
> This looks okay. This is probably the best we can achieve given the
> amount of time we can spend on this.
> 
> I started with the review thinking that the client context functions
> should get their own .c/.h files only linked by clients. This very
> likely would mean that the same has to happen for the
> popt_common_credentials_callback. Grepping for POPT_COMMON_CREDENTIALS
> looks okay in this regard. I know this opens a huge can of worms, but
> couldn't this be a chance to clean up that mess a bit?

I have been looking at this a bit more. It seems like part of the
complexity came from two commits that moved common calls from cmdline
tools to the popt_common_credentials callback:

d6d889 s3:popt_common: simplify popt_common_credentials handling
4fb0e6 s3:popt_common: let POPT_COMMON_CREDENTIALS imply logfile and conffile loading

Sharing the common code is good, but i am wondering if the post hook in
popt is the best place for that. Right now i am exploring whether a
cleaner approach would be:
 
 - Move the complex calls from the post hook to a helper function that
   can be called from the cmdline tools (there is already
   source3/lib/util_cmdline.c which might be a good place).
 - Introduce a new cmdline_contexts.c, similar to server_contexts, but
   with the additional checks for cmdline tools.
 - That would require introducing a "set" function for the messaging
   context used in db_open.

Christof



More information about the samba-technical mailing list