Re: More review food -- get rid of inbuf/outbuf

[Jeremy Allison]

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.