[PATCH] Have the smbcli_session record the OS and Native LANMAN of the remote server

[Stefan (metze) Metzmacher]

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