Re: [bug-inetutils] [patches 1,2,3] Making tftp/tftpd IPv6-capable.

[Alfred M. Szmidt]

   After some workarounds, the intended IPv6-functionality is also
   present for OpenBSD. The quirks are due to 'src/logger.c' and
   'libinetutils/utmp_init.c and 'libinetutils/utmp_logout.c', but do
   not touch tftp, nor tftpd.

Sweet!  When your papers are in order, I'll commit this and fix the
below mentioned notes before doing so.

      tftp_related_patch_1.diff: Adds functionality to 'libinetutils'.
                                 Both later patches depend on this first one.

      tftp_related_patch_2.diff: Implements changes to 'src/tftpd.c'.

      tftp_related_patch_3.diff: Likewise for 'src/tftp.c'.

   The latter two are independent of each other (apart from touching ChangeLog).

Excellent!

   --- /dev/null
   +++ b/libinetutils/sockaddr_aux.c
   @@ -0,0 +1,98 @@
   +/*
   +  Copyright (C) 2010 Free Software Foundation, Inc.
[...]
   +*/
   +
   +/* A collection of helpers intended to handle IPv6 and IPv4 simultaneously
   +   in a transparent manner. An underlying use of 'struct sockaddr_storage'
   +   is conceiled as 'struct sockaddr' in the call from an application.
   +
   +   The helper function resolves and manipulates this as 'struct sockaddr_i=
   n'
   +   or as 'struct sockaddr_in6', depending on contents.
   +
   +   Mats Erik Andersson, August 2010
   +*/

I'd suggest that you add a `Written by ....' line, and skip the date.
The date is obvious from the copyright notice.  Also, seeing that
these are new files, we should add a single line explaining what the
file is for on the first line.

   +/*
   + * get_port
   + *
   + * Return: port number in host byte order.
   + * Input:  pointer to 'struct sockaddr'.
   + */
   +in_port_t
   +get_port (struct sockaddr *sa)
   +{

The comment is kinda duplicating the code, a better comment might just
be "Return port number in host byte order", there is no need to
mention what the function is called, what kind of input it takes, or
what it returns.  It is better to simply explain its intended
function.

   diff --git a/ChangeLog b/ChangeLog
   index af7429b..cbd3db2 100644
   --- a/ChangeLog
   +++ b/ChangeLog
   @@ -1,5 +1,9 @@
    2010-08-19  Mats Erik Andersson <address@hidden>

   +    * src/tftpd.c: Implement full support for IPv6.

This is a common mistake when writting ChangeLog entries, one should
write what is actually changed in the file.  For example, for tftpd.c
we include a new header, changed the type of FROM, etc.  So it should
look like this:

2010-08-19  Mats Erik Andersson <address@hidden>

        * src/tftpd.c: Include "sockaddr_aux.h".  Changed type of `from' to 
`struct sockaddr_storage'.
        (verifyhost): Changed type of function to `static const char
        *verifyhost (struct sockaddr_storage *)' and updated all
        calees.
        (main): ...

Other than that, lovley!