More review food -- get rid of inbuf/outbuf

Jeremy Allison jra at samba.org
Tue Jul 10 22:30:22 GMT 2007


On Tue, Jul 10, 2007 at 11:10:04PM +0200, Volker Lendecke wrote:
> Hi, Jeremy!
> 
> Under http://samba.org/~vlendec/inbuf/ find my current
> patches. #0038 is the last one that prepares for a different
> reply_xx API, this series begins with #0032. The new
> routines will just have 

Sorry, I have a small problem with this patch :

http://samba.org/~vlendec/inbuf/0004-unix_convert-pstring-dirpath-char.patch

I don't like this hunk :

@@ -280,11 +282,11 @@ NTSTATUS unix_convert(connection_struct *conn,
 		/* The name cannot have a component of "." */
 
 		if (ISDOT(start)) {
-			if (!end)  {
-				/* Error code at the end of a pathname. */
-				return NT_STATUS_OBJECT_NAME_INVALID;
-			}
-			return determine_path_error(end+1,
			allow_wcard_last_component);
+			result = (end == NULL)
+				? NT_STATUS_OBJECT_NAME_INVALID
+				: determine_path_error(
+					end+1, allow_wcard_last_component);
+			goto fail;
 		}
 
 		/* The name cannot have a wildcard if it's not

I think the :

+                       result = (end == NULL)
+                               ? NT_STATUS_OBJECT_NAME_INVALID
+                               : determine_path_error(
+                                       end+1, allow_wcard_last_component);
+                       goto fail;

just looks too complex. Can you split it out to something like :

			if (!end)  {
				/* Error code at the end of a pathname. */
				result = NT_STATUS_OBJECT_NAME_INVALID;
			} else {
				result = determine_path_error(end+1,
					allow_wcard_last_component);
			}
			goto fail;

I know it's the same code, but I find the second version *much* *much*
easier to read than the '? :' version. Plus it keeps the comment, which
must have been important else I wouldn't have added it :-).

The rest of the patch looks very nice though - damn good work !

Jeremy.


More information about the samba-technical mailing list