Rafael Zalamena <rzalam...@gmail.com> writes:
> I'm implementing some features for dhcrelay and to make them fit I need
> some clean ups in the dhcrelay(8) first. This diff changes most of the
> input/output functions prototypes to take one parameter with all addresses
> instead of passing multiple parameters.
>
> Basically this will make input functions gather more information (source/
> destination MACs, source/destination IPs, source/destination ports) and
> use it in the output instead of trying to figure out this information along
> the way.
>
> With this we will be able to add IPv6 support and layer 2 relaying.
Nice. :)
[...]
> ok?
This conflicts with a diff that has been committed by patrick@, you'll
need to refresh it.
I didn't review it entirely, but please address the point below.
[...]
> Index: dhcpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/dhcrelay/dhcpd.h,v
> retrieving revision 1.15
> diff -u -p -r1.15 dhcpd.h
> --- dhcpd.h 7 Dec 2016 13:19:18 -0000 1.15
> +++ dhcpd.h 7 Dec 2016 13:44:35 -0000
> @@ -42,15 +42,28 @@
> #define SERVER_PORT 67
> #define CLIENT_PORT 68
>
> +/* Maximum size of client hardware address. */
> +#define CHADDR_SIZE 16
> +
> +struct packet_ctx {
> + uint8_t pc_htype;
> + uint8_t pc_hlen;
> + uint8_t pc_smac[CHADDR_SIZE];
> + uint8_t pc_dmac[CHADDR_SIZE];
> +
> + struct sockaddr_storage pc_sss;
> + struct sockaddr_storage pc_dss;
This doesn't strike me as meaningful variable names. Could you at least
replace "s" with "src" and "d" with "dst"? The purpose of the storage
seems more valuable to me than its type...
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
