On Fri, 4 Jul 2014 09:26:04 +0200
Kenneth Westerback <[email protected]> wrote:
>> @@ -847,7 +847,7 @@ parse_cidr(FILE *cfile, unsigned char *a
>> if (token == TOK_NUMBER)
>> convert_num(prefix, val, 10, 8);
>>
>> - if (token != TOK_NUMBER || *prefix < 1 || *prefix > 32) {
>> + if (token != TOK_NUMBER || *prefix > 32) {
>> parse_warn("Expecting CIDR prefix length, got '%s'", val);
>> goto nocidr;
>> }
>
> This may be correct, as my re-reading of RFC 3442 does not forbid a
> prefix of 0.
Specifying the default router in "Classless Static Routes option"
seems to be intended. See "DHCP Server Administrator
Responsibilities" section of RFC 3442.
| Many clients may not implement the Classless Static Routes option.
| DHCP server administrators should therefore configure their DHCP
| servers to send both a Router option and a Classless Static Routes
| option, and should specify the default router(s) both in the Router
| option and in the Classless Static Routes option.
> This check may be present to ensure our dhclient does not get
> confused, which I will check.
As my reading the code, it seems to be ok.
>> @@ -1156,8 +1156,9 @@ void parse_option_param(cfile, group)
>> return;
>> tree = tree_concat(tree, tree_const(&cprefix,
>> sizeof(cprefix)));
>> - tree = tree_concat(tree, tree_const(buf,
>> - sizeof(buf)));
>> + if (cprefix > 0)
>> + tree = tree_concat(tree,
>> tree_const(buf,
>> + cprefix / NBBY));
>
> Even if the idea is correct, and it seems to be, I'm pretty sure
> 'cprefix/NBBY' is not correct. e.g. if cprefix is 7, will cprefix /
> NBBY not be 0 instead of 1?
Oops, you're right.
> Perhaps '(cprefix + 7) / 8'? This would also get rid of NBBY which I'm
> leery of in this context.
I noticed existing code of dhclient and dhcpd use just 8. So I agree
getting rid of NBBY.
Updated the diff
ok?
Index: usr.sbin/dhcpd/confpars.c
===================================================================
RCS file: /cvs/src/usr.sbin/dhcpd/confpars.c,v
retrieving revision 1.22
diff -u -p -r1.22 confpars.c
--- usr.sbin/dhcpd/confpars.c 21 Jan 2014 03:07:51 -0000 1.22
+++ usr.sbin/dhcpd/confpars.c 4 Jul 2014 12:51:57 -0000
@@ -823,7 +823,7 @@ void parse_group_declaration(cfile, grou
* bit-count :== 0..32
*/
int
-parse_cidr(FILE *cfile, unsigned char *addr, unsigned char *prefix)
+parse_cidr(FILE *cfile, unsigned char *addr, unsigned char *prefix)
{
char *val;
int token;
@@ -835,7 +835,7 @@ parse_cidr(FILE *cfile, unsigned char *a
parse_warn("Expecting CIDR subnet");
goto nocidr;
}
-
+
token = next_token(&val, cfile);
if (token != '/') {
parse_warn("Expecting '/'");
@@ -847,7 +847,7 @@ parse_cidr(FILE *cfile, unsigned char *a
if (token == TOK_NUMBER)
convert_num(prefix, val, 10, 8);
- if (token != TOK_NUMBER || *prefix < 1 || *prefix > 32) {
+ if (token != TOK_NUMBER || *prefix > 32) {
parse_warn("Expecting CIDR prefix length, got '%s'", val);
goto nocidr;
}
@@ -1156,8 +1156,9 @@ void parse_option_param(cfile, group)
return;
tree = tree_concat(tree, tree_const(&cprefix,
sizeof(cprefix)));
- tree = tree_concat(tree, tree_const(buf,
- sizeof(buf)));
+ if (cprefix > 0)
+ tree = tree_concat(tree, tree_const(buf,
+ (cprefix + 7) / 8));
break;
default:
warning("Bad format %c in parse_option_param.",