[PATCH] smbget : enable update and various cleanup

Volker Lendecke Volker.Lendecke at SerNet.DE
Mon Feb 25 20:56:33 GMT 2008


On Mon, Feb 25, 2008 at 08:58:22PM +0100, neologix at free.fr wrote:
> Alright. Round 2 :-)
> You'll find 2 patches.
> The first one is cleanup: I made static some variables that were unnecessarily
> glocal, wrote a header, prototypes, corrected indentation, etc.

Thanks! That makes comments easier.

I like the "update" patch. If that only applied individually :-)

I've got some comments on the restructuring patch which I
don't really like as much. I do agree that smbget is ugly
and pure reformatting could do it a lot good, but there are
some points about your patch:

"mode" is still global. Why change individual globals to one
big one? I could see it if you passed that struct as a
parameter, but replacing one global with another -- why?

Why did you change the return value of smb_download_dir?
What's so bad about the convention

if (!smb_download_dir(bla)) {
	error();
}

?

For example the hunk starting with 374. If you do all the
reformatting, why not there? And why not follow the
convention to not go beyond 80 lines?

Sorry to be picky, but right now I don't see the structural
benefit of the changes you made.

Volker
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba-technical/attachments/20080225/aa512cdc/attachment.bin


More information about the samba-technical mailing list