[PATCH] fix an outdated comment

[Michael Adam]

Hi Ira,

thanks for your comment!

On 2014-07-29 at 09:57 -0400, Ira Cooper wrote:
> (found via: grep -nr)

(I usually use "git grep".)

> lib/util/debug.h:48:#define DEBUGLEVEL DEBUGLEVEL_CLASS[DBGC_ALL]
> 
> So... I think that's external.

Right, but is is not a variable!
DEBUGLEVEL used to be a variable in this module.
If you check 3.5 code you see this:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/* -------------------------------------------------------------------------- **
 * External variables.
 *
 *  dbf           - Global debug file handle.
 *  debugf        - Debug file name.
 *  DEBUGLEVEL    - System-wide debug message limit.  Messages with message-
 *                  levels higher than DEBUGLEVEL will not be processed.
 */

XFILE   *dbf        = NULL;
static char *debugf = NULL;
bool    debug_warn_unknown_class = True;
bool    debug_auto_add_unknown_class = True;
bool    AllowDebugChange = True;

/*
   used to check if the user specified a
   logfile on the command line
*/
bool    override_logfile;

/*
 * This is to allow assignment to DEBUGLEVEL before the debug
 * system has been initialized.
 */
static int debug_all_class_hack = 1;
static bool debug_all_class_isset_hack = True;

static int debug_num_classes = 0;
int     *DEBUGLEVEL_CLASS = &debug_all_class_hack;
bool    *DEBUGLEVEL_CLASS_ISSET = &debug_all_class_isset_hack;

/* DEBUGLEVEL is #defined to *debug_level */
int     DEBUGLEVEL = &debug_all_class_hack;


/* -------------------------------------------------------------------------- **
 * Internal variables.
 *
 ...

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

So you see it _was_ a variable at that time
but the comment was outdated already...

> The other one, looks like it became
> state.debugf which is defined in the structure right above... so it feels a
> bit redundant.

Exactly. That's why I wanted to remove the legacy comment.
These are sections about the global variables in the module,
as it seems, one about the external ("exported") and one
about the internal variables.

> Personally, if I touched that comment, I'd remove the whole block.

I think it is not that easy:
As the above code snippet shows, this is simply
a poorly formatted/grown comment/declaration section and the "External
variables" section is (if I got that right) intended to stretch
up to the beginning of the "Internal variables" comment.
I think this was once started as a comment block explaining
each variable followed by the variables without further comment,
and then the variables were worked on, some having their own
comments now and the main header not being updated.

That's why I did not remove it completely:
We now still have the in the external variables section:
override_logfile, debug_num_classes, debug_class_list_initial,
DEBUGLEVEL_CLASS.

Of these, override_logfile is directly used externally, and
DEBUGLEVEL_CLASS is used indirectly by the macros from
lib/util/debug.h, like DEBUGLEVEL.
So these two really qualify as external.

But debug_num_classes and debug_class_list_initial are static,
so not precisely external. :-) So these should qualify as
internal, but at least debug_class_list_initial is needed for
DEBUGLEVEL_CLASS...

Not sure what to do to clean this up better. I initially wanted
to only remove the (as I thought) obviously wrong stuff. :-)

Anyhow: I would like to keep up my patch as an initial step
of cleaning this up a bit.

Any further thoughts?

Cheers - Michael

> -Ira
> 
> 
> On Tue, Jul 29, 2014 at 9:17 AM, Michael Adam <obnox at samba.org> wrote:
> 
> > Attached find a patch to fix an outdated comment in debug.c
> >
> > I stubled over it while investigating some issue with our
> > debug / logfiles.
> >
> > review/comments/push appreciated,
> >
> > Cheers - Michael
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140729/a4a4c050/attachment.pgp>