need some insight in udhcp structures size

[Cristian Ionescu-Idbohrn]

On Fri, 23 Nov 2007, Denys Vlasenko wrote:

> > > +   length = sizeof(packet->options);
> >
> > Yes.  I'll do that on the magic numbers.
>
> It's already done in svn. Take a look.

Brilliant ;)

I did look through and found a few more places.

The 576 is still magic.  I think it would be better with something
like:

#define UDP_DHCP_PACKET_MIN_SIZE 576

./common.h:52:                [sizeof(struct udp_dhcp_packet) != 576 ? -1 : 1];

Other places where sizeof() may be more appropriate:

./dhcpd.c:158:			memcpy(&static_lease.chaddr, &packet.chaddr, 16);
./dhcpd.c:217:						memset(lease->chaddr, 0, 16);
./dhcpd.c:238:				memset(lease->chaddr, 0, 16);

./leases.c:33:	for (j = 0; j < 16 && !chaddr[j]; j++)
./leases.c:37:		if ((j != 16 && memcmp(leases[i].chaddr, chaddr, 16) == 0)
./leases.c:56:		memcpy(oldest->chaddr, chaddr, 16);
./leases.c:78:		if (!memcmp(leases[i].chaddr, chaddr, 16))
./leases.c:101:	static const uint8_t blank_chaddr[16]; /* 16 zero bytes */

The 308 is magic too.  A define for that too?

./options.c:148:	if (end + string[OPT_LEN] + 2 + 1 >= 308) {

> > * what you suggest: bump the size of options, or
> > * add a pad-variable to the structure
>
> First. This way, sizeof() of options field will give correct value,
> and no code changes will be needed.

Agreed.

> > I thought of doing that with a configuration option instead of hard
> > coding.  What do you think?
>
> Config options are needed when people may want to exclude functionality
> and save on code size. Here code size will be the same.

Still...  This tweak is needed _only_ if the dhcp-server is brain
dead, problem which most clients don't suffer of.


Cheers,

-- 
Cristian