On 6/24/05, abe-t <[EMAIL PROTECTED]> wrote:
> Update of /cvsroot/naviserver/naviserver/nsd
> In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv18008/nsd
> 
> Modified Files:
>         return.c
> Log Message:
>   Added Ns_ConnReturnPath to open a file and send its contents
>     via the connection.
> 
> 
> Index: return.c
> ===================================================================
> RCS file: /cvsroot/naviserver/naviserver/nsd/return.c,v
> retrieving revision 1.5
> retrieving revision 1.6
> diff -C2 -d -r1.5 -r1.6
> *** return.c    11 Jun 2005 20:04:11 -0000      1.5
> --- return.c    24 Jun 2005 08:42:02 -0000      1.6
> ***************
> *** 1362,1363 ****
> --- 1362,1397 ----
>       return result;
>   }
> +
> + /*
> +  *----------------------------------------------------------------------
> +  *
> +  * Ns_ConnReturnPath --
> +  *
> +  *  Open a path and send contents out the conn.
> +  *
> +  * Results:
> +  *  See ReturnOpen.
> +  *
> +  * Side effects:
> +  *  See ReturnOpen.
> +  *
> +  * Note:
> +  *  Added by Archiware
> +  *
> +  *----------------------------------------------------------------------
> +  */
> +
> + NS_EXPORT int
> + Ns_ConnReturnPath(Ns_Conn *conn, int status, char *type, char *path, int 
> len);
> +
> + int
> + Ns_ConnReturnPath(Ns_Conn *conn, int status, char *type, char *path, int 
> len)
> + {
> +     int fd = open(path, O_RDONLY|O_BINARY);
> +     int rv;
> +
> +     if (fd < 0) return -1;
> +     rv = ReturnOpen(conn, status, type, NULL, NULL, fd, len);
> +     close(fd);
> +     return rv;
> + }


I noticed a couple of problems with this change:

The function declaration should never come immediately before the
definition; you're not getting any extra checking from the compiler
this way.  Private (static) declarations go at the top of the file,
declarations private to a module go in private headers (nsd.h in this
case) and have no underscore in the prefix (NsConnReturnPath), and
public declarations go in include/ns.h which is where this one lives.

There's no 'Note:' section in the comments where you can put 'Added by
Archiware'.  Be verbose in the commit message/ChangeLog if you must.

The body of an 'if' statement is always enclosed in braces. Do only
simple initialisation of variables in the variable declaration block. 
You can pick this up by looking at existing code, but here's our new
style guidelines doc, such as it is:

http://naviserver.sourceforge.net/wiki/index.php/Code_Standards


Finally, is this function necessary?  Ns_ConnReturnFile already exists
in fastpath.c.

Reply via email to