including samba vfs module scannedonly in source tree

Volker Lendecke Volker.Lendecke at SerNet.DE
Mon Nov 23 15:55:16 MST 2009


On Mon, Nov 23, 2009 at 10:18:05PM +0100, Olivier Sessink wrote:
> Could you take a look at the code and see if things go in the right
> direction? If so I'll branch, remove all the 3.0/3.2/3.4 compatibility
> and create a 3.5-only version.

Yep, that looks better. Thanks!

Some more comments. Some of them are hints to make the code
more readable, they would not be blockers for inclusion.
Some are (IMHO) bugs.

For example, one thing that helps in avoiding deep nesting
is to do early returns in functions. For example, in
construct_full_path(), the "else" branch is not required.
You always return in the "if" branch, so you can shift the
code in the "else" branch one tab left.

Line 242 is missing a "return -1"?

In line 310 you should check for the return value of
connect_to_scanner() I think.

Line 320 and 335: gcc does the clean tail recursion
optimization, so you don't grow the stack, but other
compilers don't. Can you turn that into a while loop?

Line 342: We have "bool" for free_tmp.

Line 436: Turning that into

if ((retval == 0) && (sbuf1.st_mtime <= sbuf2.st_mtime)) {
	SAFE_FREE(cachefile);
	return 1;
}

reversing the if-condition and doing an early return gets
rid of one indent and makes cleaner code. Similar for line
566 for example.

Line 830: Reverse the condition and do a continue;

Volker
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091123/23e2a331/attachment.pgp>


More information about the samba-technical mailing list