[PATCH] Have the smbcli_session record the OS and Native LANMAN
of the remote server
Stefan (metze) Metzmacher
metze at samba.org
Fri May 1 14:15:36 GMT 2009
Amin Azez schrieb:
> * Gerald Carter wrote, On 29/04/09 17:32:
>>> There's more than one established convention widely
>>> present in the code base.
>> Agreed. But this should not really be a question of
>> personal style. If it matches README.Coding and
>> prog_guide4.txt you should be safe. If you follow
>> that and the patch is refused due to style reasons,
>> it is the project's fault and not yours.
>
> I'm not picking fault here, or trying to argue; just pointing out how it
> looks from my position.
>
> The documents you mention encourage the style I used, and do not
> discourage it, apart from tab-8 spacing which no-one follows.
>
> The two documents you mention make recommendations that are not followed
> even in new code.
>
> In short, they can't be trusted, it seems that they misled me and are
> somewhat ignored by everyone else.
>
>
>
> prog_guide4.txt
>
> I've read prog_guide4.txt numerous times before before and re-read it today.
>
> It's still only "solicit[ing] comments from a few" and the only
> reference to the aspects of style we were discussing is a preference for
> another document elsewhere: Documentation/CodingStyle in the linux kernel.
>
>
>
> Documentation/CodingStyle
>
> Recommends 8-space tabs, but samba uses 4 spaces.
>
> Encourages use of braces-less if statements, e.g.
>
> | if (buffer == NULL)
> | return -ENOMEM;
>
> We regularly break the recommendation about not returning from a
> function via a macro. (822 times by my count)
>
> Against the advice of this document, we have loads of macros that depend
> on local variables having certain names.
>
> To my mind both of this "infractions" are fine, and are probably what
> README.Coding refers to as "coding style should never outweigh coding
> itself"
>
>
>
> README.Coding
>
> It refers to Documentation/CodingStyle again and K&R guidelines (where
> ever they are?) and humourously suggests an automatic re-formatter,
> later disparaged by prog_guide4.txt
>
> It is also out of date, It suggests 8 space tabs to indent, but 4 spaces
> are used nearly everywhere.
>
> Finally, it explicitly encourages the style I was using:
>
> (reformatted)
> |When invoking functions that return pointer values, either of the
> |following are acceptable. Use you best judgement[1] and choose the
> |more readable option.
> |Remember that many other people will review it.::
> |
> | if ((x = malloc(sizeof(short)*10)) == NULL ) {
> | fprintf(stderr, "Unable to alloc memory!\n");
> | }
> |
> |or::
> |
> | x = malloc(sizeof(short)*10);
> | if (!x) {
> | fprintf(stderr, "Unable to alloc memory!\n");
> | }
>
>
>
> So I've broken the "secret" Samba team coding style, by following the
> recommendations of the published coding style documents, which even says
> (README.Coding):
>
> "However, coding style should never outweigh coding itself..."
>
> [1] In my judgement I used the more readable option; however I
> acknowledge that many other people review it, but compare the attached
> 51 line patch (48 lines to copy 6 strings!) and see which is more
> readable - I mean readable-for-intent, not human-as-a-compiler.
>
> Maybe a third way with this macro? We're certainly not afraid of macros
> in samba:
We clearly need to combine, update and improve our coding guides.
At least for me it would make sense to have a rule to
- do only one thing in one line.
- don't assign variables within if statements
- don't call functions as function parameters, use helper variables.
functiona(functionb()) is bad
- prefer using
if (x == y) {
return;
}
instead of
if (x == y)
return;
metze
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 252 bytes
Desc: OpenPGP digital signature
Url : http://lists.samba.org/archive/samba-technical/attachments/20090501/448c80b4/signature.bin
More information about the samba-technical
mailing list