[naviserver-devel] Re: [naviserver-commits] naviserver/nsd return.c,1.5,1.6

[Stephen D. <sd...@gm...>]

On 6/24/05, abe-t <it...@us...> wrote:
> Update of /cvsroot/naviserver/naviserver/nsd
> In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv18008/nsd
>=20
> Modified Files:
>         return.c
> Log Message:
>   Added Ns_ConnReturnPath to open a file and send its contents
>     via the connection.
>=20
>=20
> Index: return.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> 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, in=
t len);
> +
> + int
> + Ns_ConnReturnPath(Ns_Conn *conn, int status, char *type, char *path, in=
t len)
> + {
> +     int fd =3D open(path, O_RDONLY|O_BINARY);
> +     int rv;
> +
> +     if (fd < 0) return -1;
> +     rv =3D 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.=20
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.