[PATCH] Fix the build

Christof Schmitt cs at samba.org
Fri Sep 2 16:28:36 UTC 2016


On Fri, Sep 02, 2016 at 09:34:49AM +0200, Volker Lendecke wrote:
> Hi!
> 
> Review appreciated!
> 
> Looking at commit 1c636532874da from a few weeks ago I begin to question
> the value of our README.Coding file. I've asked a few times to fix
> patches to follow the 80-column rule, I even provided patches to assist.
> 
> There's a reason why we have this rule: It's not that we are all sitting
> at 3270 or vt100 terminals. We want to avoid arbitrarily deeply nested
> control structures. It might be more work, but well-named factored
> out subfunctions foster unterstanding of complex code. Looking at
> dsdb_garbage_collect_tombstones(), we have four (!!)  levels of nested
> for-loops. One line I've just come across almost touches twice the
> 80-columns with its length of 157 chars.
> 
> So, shall we drop the README.Coding section on 80 chars, as it is not
> generally seen as worthwhile following?

No. There are lots of places that do not adhere to the 80 character
limit, but any changed code should adhere to the limit. An editor can
usually highlight the long lines easily (we could suggest settings for
common editors in the wiki). And i agree with the point that long lines
can indicate a need to factor out some code to subfunctions.

So +1 for keeping this coding rule, and possibly enforcing it.

Christof

> 
> Volker

> From ff62a5947cea01f02186e658105bb870a90080c5 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Fri, 2 Sep 2016 09:17:33 +0200
> Subject: [PATCH] kcc: Fix a -Werror,-Wformat-security error
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source4/dsdb/kcc/garbage_collect_tombstones.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/source4/dsdb/kcc/garbage_collect_tombstones.c b/source4/dsdb/kcc/garbage_collect_tombstones.c
> index 8d8a51f..8d2ea8b 100644
> --- a/source4/dsdb/kcc/garbage_collect_tombstones.c
> +++ b/source4/dsdb/kcc/garbage_collect_tombstones.c
> @@ -149,7 +149,7 @@ NTSTATUS dsdb_garbage_collect_tombstones(TALLOC_CTX *mem_ctx,
>  			DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT |
>  			DSDB_SEARCH_REVEAL_INTERNALS;
>  		ret = dsdb_search(samdb, tmp_ctx, &res, part->dn, LDB_SCOPE_SUBTREE,
> -				  attrs, flags, filter);
> +				  attrs, flags, "%s", filter);
>  
>  		if (ret != LDB_SUCCESS) {
>  			*error_string = talloc_asprintf(mem_ctx, "Failed to search for deleted objects in %s: %s",
> -- 
> 2.1.4
> 




More information about the samba-technical mailing list