Bugs in libsmbclient (was Re: Are code assertions considered harmful?)

[Richard Sharpe]

On Sun, 9 Nov 2003, Cameron Paine wrote:

> My observation was intended as a discussion point. Specific
> instances in the client library code are to be the subject of
> some future patch submissions. However, lest you think I'm
> ducking and weaving, here's a few, albeit trivial, examples.
> 
> smbc_init:
> 
> 1. ignores its logging argument
> 2. sets errno and returns -1 on most errors.
> 3. returns an errno.h constant if it cannot allocate
> the file table.
> 
> smbc_open:
> 
> 1. sets errno and returns -1 on most errors.
> 2. returns 1 if it falls out the bottom (currently it can't
> do that but...)
> 3. returns a descriptor on success
> 
> smbc_getatr (a function declared as returning a BOOL):
> 
> 1. sets errno and returns -1 on one error.
> 2. doesn't set errno and returns False on another error.
> 3. returns True on success.

These should be logged as bugs at bugzilla.samba.org. That will ensure 
that your findings are not forgotten and a larger pool of people can work 
on them. 

If you have tests that expose these bugs, it would be useful to provide 
the code for the tests as well.
 
> I have four failure modes; one we've discussed (you may recall I
> forwarded you some session captures):
> 
> Memory management. I was losing 16-20 KB every time I visited
> a server. I know memory is cheap these days, but... :-) I
> found a fix proposed to this mailing list by "Ong Kian Win"
> (codegrunt at rubbercookie.com) back in Feb 2002. So far, it has
> worked for me. I'm bothered though that you declined to
> integrate it. Why was that? What side-effects await me?
> 
> Inconsistent opendir returns. This is the one I referred to you.
> Essentially smbc_opendir returns varying numbers of files from
> a share that is not changing. I have a hack in place that's
> keeping this end of the universe propped up until I get advice
> from you. It's not (currently) urgent but I don't want to give
> my customers a solution that contains a hack that I don't
> understand.
> 
> Silent death. The process simply dies. The frequency varies. By
> carefully adding additional trace messages I'm generating logs
> that are starting to point to the problem. I've narrowed it down
> to clientgen.c:cli_send_smb(). This is a work in progress.
> 
> Transport layer errors don't propagate upwards. When this one
> bites I cannot continue transactions with the target server
> unless I shut down the process. There seems to be some persistent,
> per-server state that is maintained that I won't pretend to
> understand.
> 
> I've added enough trace code to let me identify the "trigger"
> event. I don't fully understand the implication of what I'm
> about to tell you. A typical scenario goes like this:
> 
> libsmb/libsmbclient.c:smbc_opendir()
>   libsmb/clilist.c:cli_list_new()
>     libsmb/clitrans.c:cli_receive_trans()
>       libsmb/clientgen.c:cli_receive_smb()
>         lib/util_sock.c:client_receive_smb()
>           lib/util_sock.c:receive_smb()
>             lib/util_sock.c:read_smb_length_return_keepalive()
>               lib/util_sock.c:read_socket_with_timeout()

Please log these as well.

Regards
-----
Richard Sharpe, rsharpe[at]ns.aus.com, rsharpe[at]samba.org, 
sharpe[at]ethereal.com, http://www.richardsharpe.com