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

Sam Liddicott sam at liddicott.com
Wed Apr 29 09:08:36 GMT 2009


* Stefan (metze) Metzmacher wrote, On 29/04/09 09:32:
> Sam Liddicott schrieb:
>   
>> Volker, hows this then?
>>
>>
>> Signed-off-by: Sam Liddicott <sam at liddicott.com>
>> ---
>>  source4/libcli/raw/libcliraw.h           |    3 +++
>>  source4/libcli/smb_composite/sesssetup.c |   12 ++++++++++++
>>  2 files changed, 15 insertions(+), 0 deletions(-)
>>
>>
>>
>>     
> +		if (state->setup.nt1.out.os &&
> +			!(session->os=talloc_strdup(session, state->setup.nt1.out.os)))
> c->status = NT_STATUS_NO_MEMORY;
> +		if (state->setup.nt1.out.lanman &&
> +			!(session->lanman=talloc_strdup(session,
> state->setup.nt1.out.lanman))) c->status = NT_STATUS_NO_MEMORY;
>
> why do you want to do everything in one line?
>   
Because I think it is easier to construe the intention, rather than
merely the action:

if (state->setup.nt1.out.os &&
   !(session->os=talloc_strdup(session, state->setup.nt1.out.os)))
       c->status = NT_STATUS_NO_MEMORY;

spead reading: If there is an out.os and you can't save it then fail.

In longhand it says:
If state's setup nt1 out has an os but talloc_strdup fails to copy it to
the session os then store the fault in c->status

- which still expresses the intention directly.
> if (state->setup.nt.out.os) {
>       session->os = talloc_strdup(session, state->setup.nt1.out.os);
>       if (!session->os) {
>             c->status = NT_STATUS_NO_MEMORY;
>       }
> } else {
>       sesson->os = NULL;
> }
>   
The above disguises the intention in a tedium of small operations,
taking 8 lines, only two of which represent common execution behaviour.

There's too little meaning, too little going on to spread over eight lines.

> would be much nicer and much easier to read and understand.
>   
Well... I think it's very tedious and wastes a lot of screen space
drawing attention to something that is a very minor action for the
function. I don't want it to look more than a simple safe-copy, or it
disturbs the reading of the function.

I prefer a fractal-detail approach. My one liner has just as much
structure in it, but you don't need to see the structure until you look
at it in detail.

It's then easier to see the function's intended at a glance without
being caught up in details which are important but don't represent the
intent of the function.

Sam


More information about the samba-technical mailing list