Using dbgtext for logging

[Christopher R. Hertel]

Dan,

Many, many years ago I overhauled the debug subsystem in Samba.  I provided
a more complete API, including the dbgtext() function, and I went through
the Samba source and found instances in which we were calling DEBUG macros
multiple times to compose long messages.  In those cases, I replaced the
multiple macro calls with what I called "Debug blocks", similar to what you
see below.  Some others also picked up the style.

The advantage of doing it that way was--and is--that there is only one if()
statement surrounding the block, and the programmer has the ability to do
more careful formatting of the output.  In addition, because it's a clear
block, temporary stack variables can be allocated and calculations that only
pertain to the block can be performed.  Those variables are dropped when the
block is exited.

...but it wasn't common style for most of the Team so, over time, much of
the code has been reverted to the simpler style.  In addition, the debug
subsystem has been rewritten a few times since.

So what you're seeing here is an artifact dating back to the Samba 2.2.x
days.  It's not a bug, it's just an inconsistency.

Chris -)-----

On 07/31/2013 04:00 AM, Dan Cohen1 wrote:
> Hi,
> 
> While doing some work on samba logging I've noticed that there are several 
> places in the samba code where logging is done directly using the dbgtext 
> function. For example: in source3/smbd/oplock.c:initial_break_processing 
> (line 267):
> 
> static files_struct *initial_break_processing(
>     struct smbd_server_connection *sconn, struct file_id id,
>     unsigned long file_id)
> {
>     files_struct *fsp = NULL;
> 
>     if( DEBUGLVL( 3 ) ) {
>         dbgtext( "initial_break_processing: called for %s/%u\n",
>              file_id_string_tos(&id), (int)file_id);
>         dbgtext( "Current oplocks_open (exclusive = %d, levelII = %d)\n",
>             sconn->oplocks.exclusive_open,
>             sconn->oplocks.level_II_open);
>     }
> 
>     /*
>      * We need to search the file open table for the
>      * entry containing this dev and inode, and ensure
>      * we have an oplock on it.
>      */
> 
>     fsp = file_find_dif(sconn, id, file_id);
> 
>     if(fsp == NULL) {
>         /* The file could have been closed in the meantime - return 
> success. */
>         if( DEBUGLVL( 3 ) ) {
>             dbgtext( "initial_break_processing: cannot find open file with 
> " );
>             dbgtext( "file_id %s gen_id = %lu", file_id_string_tos(&id), 
> file_id);
>             dbgtext( "allowing break to succeed.\n" );
>         }
>         return NULL;
>     }
> 
>     /* Ensure we have an oplock on the file */
> 
>     /*
>      * There is a potential race condition in that an oplock could
>      * have been broken due to another udp request, and yet there are
>      * still oplock break messages being sent in the udp message
>      * queue for this file. So return true if we don't have an oplock,
>      * as we may have just freed it.
>      */
> 
>     if(fsp->oplock_type == NO_OPLOCK) {
>         if( DEBUGLVL( 3 ) ) {
>             dbgtext( "initial_break_processing: file %s ",
>                  fsp_str_dbg(fsp));
>             dbgtext( "(file_id = %s gen_id = %lu) has no oplock.\n",
>                  file_id_string_tos(&id), fsp->fh->gen_id );
>             dbgtext( "Allowing break to succeed regardless.\n" );
>         }
>         return NULL;
>     }
> 
>     return fsp;
> }
> 
> Calling directly to dbgtext happens also in a few other files such as 
> source3/smbd/service.c, source3/smbd/posix_acl.c and more.
> This method of logging is on contrary to most of the samba code where 
> logging is done using the logging macros DEBUG, DEBUGC etc.
> 
> Is avoiding the log macros done in those files deliberately? Is it done 
> from any particular reason? Or is it just leftovers from old code or 
> something like that?
> 
> Thanks,
> Dan Cohen
> IBM - XIV, Israel
> NAS Development Team
> 

-- 
"Implementing CIFS - the Common Internet FileSystem" ISBN: 013047116X
Samba Team -- http://www.samba.org/     -)-----   Christopher R. Hertel
jCIFS Team -- http://jcifs.samba.org/   -)-----   ubiqx development, uninq.
ubiqx Team -- http://www.ubiqx.org/     -)-----   crh at ubiqx.mn.org
OnLineBook -- http://ubiqx.org/cifs/    -)-----   crh at ubiqx.org