[PATCH] smbget : enable update and various cleanup

neologix at free.fr neologix at free.fr
Mon Feb 25 22:35:08 GMT 2008


> I don't see the use for an extra header. It's only included in smbget.c
> and would only ever make sense to include there. There are also static
> variables in there, which should never be in a header.

To be clean.
Concerning the static variables, I didn't put any variable
declaration/definition in the header, only macros.

> This change looks fine mostly and I think it's almost ready to go in.
> Any chance you can send a version of this patch that doesn't depend on
> the first one?

The patch is attached.

>  "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?

It could be an option, but there's no way to pass it around like it's done with
recursive and resume, because it's used all the time (just see how many time you
pass resume or recursive in function calls).
Furthermore, I declared it as static, so it won't be exported by the linker,
contrarily to many other global variables (username, password, dots, etc). At
least, put all these variables static.

> Why did you change the return value of smb_download_dir?
> if (!smb_download_dir(bla)) {
>         error();
> }

Because in every single function/system call I know (except inet_aton) returns 0
on success, non-zero otherwise. Hey, you're the onse talking about consistency!

cheers,

Charles
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smbget_update.patch
Type: text/x-diff
Size: 2174 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba-technical/attachments/20080225/1bf5898d/smbget_update.bin


More information about the samba-technical mailing list