[PATCH] Fix the build

Michael Adam obnox at samba.org
Fri Sep 2 08:06:56 UTC 2016


On 2016-09-02 at 09:34 +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?

1000 times no!

We should rather have a check script that validates the patch.

e.g. glusterfs does this:

https://github.com/gluster/glusterfs/blob/master/rfc.sh

which is used to submit patches, calls

https://github.com/gluster/glusterfs/blob/master/extras/checkpatch.pl

We could include something similar in our autobuild receive hook
on the autobuild server. So that patches that don't adhere are
rejected. (A forceful override can be discussed.)

It is just too annoying that this is consistently ignored.
As you explained, there are good reasons beyond cosmetics.


> 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


reviewed-by: me, pushing ...

Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160902/abbb4a8c/signature.sig>


More information about the samba-technical mailing list