A question about use of DEBUG in wb_reqtrans.c

[Jeremy Allison]

On Fri, Jan 23, 2015 at 11:07:09AM +0000, Noel Power wrote:
> Hi Metz, All
> 
> Recently I was looking at a problem where the root cause would have
> easily been seen if the DEBUG messages in wb_reqtrans.c were functional.
> I missed entirely that the DEBUG macro was disabled at the top of the
> file and because of that for a while ended up on a wild goose chase ;-)
> Afterwards my first thought was if those DEBUG messages are not
> functional then it's better to remove them entirely (rather than cause
> confusion). However, it would be nicer of course if they could be enabled.
> 
> The commit that seemed to remove that functionality was
> https://git.samba.org/?p=samba.git;a=commit;h=faabc97c9adffd9468a5d1606467359a81445cf3
> 
> I wonder though if the reason disabling the DEBUG macro in this file is
> still relevant (perhaps it isn't)
> 
> looking at the current usage in master wb_reqtrans.c
> 
> wb_reqtrans.c is used in
>     a) LIBWBCLIENT_OLD (source4/libcli/wbclient/wscript_build) library
>     b) winbindd (source3/wscript_build)
>     c) smbtorture (source3/wscript_build)
> 
> LIBEBCLIENT_OLD is used
> 
> ./source4/auth/ntlm/wscript_build
> ./source4/auth/wscript_build
> ./source4/ntvfs/posix/wscript_build
> ./source4/rpc_server/wscript_build
> 
> afaics all of the targets above either already link in objects that
> already contain the DEBUG or reference DEBUG macros directly themselves
> so I don't really see an issue enabling it. So is it ok to try reenable
> the messages ?, get rid of them ? (patches for either option attached)
> or is there another alternative

So I vote for reenabling (patch #2), as the other targets already
use DEBUG.

Reviewed-by: Jeremy Allison <jra at samba.org>

> >From 11536b54971629a5af290fea28b1cbca2a57dbc5 Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Fri, 23 Jan 2015 11:01:51 +0000
> Subject: [PATCH] remove non-functional DEBUG messages
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  nsswitch/wb_reqtrans.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/nsswitch/wb_reqtrans.c b/nsswitch/wb_reqtrans.c
> index 779ef52..2063379 100644
> --- a/nsswitch/wb_reqtrans.c
> +++ b/nsswitch/wb_reqtrans.c
> @@ -34,9 +34,6 @@
>  #include "nsswitch/libwbclient/wbclient.h"
>  #include "nsswitch/wb_reqtrans.h"
>  
> -/* can't use DEBUG here... */
> -#define DEBUG(a,b)
> -
>  struct req_read_state {
>  	struct winbindd_request *wb_req;
>  	size_t max_extra_data;
> @@ -75,10 +72,6 @@ static ssize_t wb_req_more(uint8_t *buf, size_t buflen, void *private_data)
>  
>  	if (buflen == 4) {
>  		if (req->length != sizeof(struct winbindd_request)) {
> -			DEBUG(0, ("wb_req_read_len: Invalid request size "
> -				  "received: %d (expected %d)\n",
> -				  (int)req->length,
> -				  (int)sizeof(struct winbindd_request)));
>  			return -1;
>  		}
>  		return sizeof(struct winbindd_request) - 4;
> @@ -91,8 +84,6 @@ static ssize_t wb_req_more(uint8_t *buf, size_t buflen, void *private_data)
>  
>  	if ((state->max_extra_data != 0)
>  	    && (req->extra_len > state->max_extra_data)) {
> -		DEBUG(3, ("Got request with %d bytes extra data on "
> -			  "unprivileged socket\n", (int)req->extra_len));
>  		return -1;
>  	}
>  
> @@ -238,10 +229,6 @@ static ssize_t wb_resp_more(uint8_t *buf, size_t buflen, void *private_data)
>  
>  	if (buflen == 4) {
>  		if (resp->length < sizeof(struct winbindd_response)) {
> -			DEBUG(0, ("wb_resp_read_len: Invalid response size "
> -				  "received: %d (expected at least%d)\n",
> -				  (int)resp->length,
> -				  (int)sizeof(struct winbindd_response)));
>  			return -1;
>  		}
>  	}
> -- 
> 1.8.4.5
> 

> >From 0a88b682734ee33a1c2f2c5dc7cc5b3a72bc3e4b Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Fri, 23 Jan 2015 10:59:43 +0000
> Subject: [PATCH] make DEBUG macro functional again
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  nsswitch/wb_reqtrans.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/nsswitch/wb_reqtrans.c b/nsswitch/wb_reqtrans.c
> index 779ef52..de7b4ad 100644
> --- a/nsswitch/wb_reqtrans.c
> +++ b/nsswitch/wb_reqtrans.c
> @@ -23,6 +23,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  */
>  
> +#include "includes.h"
>  #include "replace.h"
>  #include "system/filesys.h"
>  #include "system/network.h"
> @@ -34,9 +35,6 @@
>  #include "nsswitch/libwbclient/wbclient.h"
>  #include "nsswitch/wb_reqtrans.h"
>  
> -/* can't use DEBUG here... */
> -#define DEBUG(a,b)
> -
>  struct req_read_state {
>  	struct winbindd_request *wb_req;
>  	size_t max_extra_data;
> -- 
> 1.8.4.5
>