[PATCH] udhcp[cd]: add -P switch for assigning server/client port
Jazz Yao-Tsung Wang
jazz at nchc.org.tw
Wed Dec 26 22:06:49 PST 2007
Hi,
This patch adds -P switch both in udhcpc.c and udhcpd.c
After stripped, it will increase 64 bytes.
$ ./scripts/bloat-o-meter busybox_1.8.2_unstripped
busybox_1.8.2_patched_unstripped
function old new delta
udhcpc_main 2405 2507 +102
udhcpd_main 1168 1267 +99
.rodata 33428 33492 +64
send_renew 129 145 +16
usage_messages 2076 2091 +15
static.udhcpc_longopts 200 214 +14
send_packet 98 111 +13
send_selecting 134 142 +8
send_release 109 117 +8
send_discover 114 122 +8
get_raw_packet 371 378 +7
server_port - 4 +4
client_port - 4 +4
------------------------------------------------------------------------------
(add/remove: 2/0 grow/shrink: 11/0 up/down: 362/0) Total: 362 bytes
$ ./scripts/bloat-o-meter busybox_1.8.2_stripped busybox_1.8.2_patched_stripped
nm: busybox_1.8.2_stripped: no symbols
nm: busybox_1.8.2_patched_stripped: no symbols
function old new delta
.rodata 33428 33492 +64
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/0 up/down: 64/0) Total: 64 bytes
Jazz Yao-Tsung Wang
2007/11/15, Denys Vlasenko <vda.linux at googlemail.com>:
> On Wednesday 14 November 2007 09:38, Jazz Yao-Tsung Wang wrote:
> > Hi,
> >
> > Here is the patch for busybox-1.8.1 which introduce new option -P for
> > udhcp[c/d].
> > We had tested it with configuration files in udhcp_configs.tar.gz.
>
>
> Unfortunately the patch needs a lot of polishing.
>
> For one, please note that bbox is trying to stay small.
> You have lots of code duplication. How many times do you
> call atoi() on port arguments?
>
>
> diff -Naur busybox-1.8.1.org/applets/applets.c busybox-1.8.1/applets/applets.c
> --- busybox-1.8.1.org/applets/applets.c 2007-11-10 09:40:53.000000000 +0800
> +++ busybox-1.8.1/applets/applets.c 2007-11-11 00:08:06.000000000 +0800
> @@ -18,7 +18,6 @@
> #warning Note that glibc is unsuitable for static linking anyway.
> #warning If you still want to do it, remove -Wl,--gc-sections
> #warning from top-level Makefile and remove this warning.
> -#error Aborting compilation.
>
>
> ??
>
>
> #define udhcpc_trivial_usage \
> "[-Cfbnqtv] [-c CID] [-V VCLS] [-H HOSTNAME] [-i INTERFACE]\n" \
> - " [-p pidfile] [-r IP] [-s script]"
> + " [-p pidfile] [-r IP] [-s script] [-P CLIENT_PORT]"
> #define udhcpc_full_usage \
> USE_GETOPT_LONG( \
> " -V,--vendorclass=CLASSID Set vendor class identifier" \
> @@ -3842,6 +3842,7 @@
> "\n -q,--quit Quit after obtaining lease" \
> "\n -R,--release Release IP on quit" \
> "\n -v,--version Display version" \
> + "\n -P,--client-port CLIENT_PORT Use CLIENT_PORT as DHCP Client port (default: 68)" \
>
> Shorter text? "-P,--client-port N Use port N instead of default 68".
>
> ) \
> SKIP_GETOPT_LONG( \
> " -V CLASSID Set vendor class identifier" \
>
> The SKIP_GETOPT_LONG case also needs "-P N" help text added.
>
>
>
> int send_discover(uint32_t xid, uint32_t requested)
> {
> struct dhcpMessage packet;
> + int SRV_PORT, CLT_PORT;
>
> ALL_CAPS are for constants and macros.
>
> +
> + SRV_PORT=(client_port == NULL)?SERVER_PORT:(atoi(client_port) - 1);
> + CLT_PORT=(client_port == NULL)?CLIENT_PORT:atoi(client_port);
>
> Don't torture yourself, propagate already-converted port numbers.
> Also validate user input - is "-345die" valid port num?
>
>
> - return udhcp_raw_packet(&packet, INADDR_ANY, CLIENT_PORT, INADDR_BROADCAST,
> - SERVER_PORT, MAC_BCAST_ADDR, client_config.ifindex);
> + return udhcp_raw_packet(&packet, INADDR_ANY, CLT_PORT, INADDR_BROADCAST, SRV_PORT, MAC_BCAST_ADDR, client_config.ifindex);
>
> It was wrapped because sometimes you need to read code on 80x24 screen.
>
>
> #include <getopt.h>
> #include <syslog.h>
> +#include <stdlib.h> // for atoi()
>
> It is already included from libbb.h.
>
> + if (opt & OPT_P) {
> + printf("Use %s as DHCP Client Port!\n",client_port);
> + }
>
> Do you mean "Using port %d\n"? Why do you need this message at all?
>
> - if (listen_mode == LISTEN_KERNEL)
> - sockfd = listen_socket(/*INADDR_ANY,*/ CLIENT_PORT, client_config.interface);
> - else
> + if (listen_mode == LISTEN_KERNEL) {
> + if (client_port == NULL) {
> + sockfd = listen_socket(/*INADDR_ANY,*/ CLIENT_PORT, client_config.interface);
> + } else {
> + sockfd = listen_socket(/*INADDR_ANY,*/ atoi(client_port), client_config.interface);
> + }
> + } else {
>
> again atoi()
>
>
> + opt = getopt32(argv, "fSP:",&server_port);
> + if ( server_port == NULL)
> + {
> + printf("Use %d for DHCP Server Port!\n",SERVER_PORT);
> + }
> + if ( server_port == NULL)
>
> Style doesn't match busybox's one. Should be "a, b", "if (expr) {..." etc
>
>
> +char *server_port = NULL;
> +char *client_port = NULL;
>
> These should be added to server_config/client_config structs instead
> (and be integers, not strings).
>
>
> + int SRV_PORT=0, CLT_PORT=0;
> +
> + SRV_PORT=(server_port == NULL)?SERVER_PORT:atoi(server_port);
> + CLT_PORT=(server_port == NULL)?CLIENT_PORT:(atoi(server_port) + 1);
>
> Useless =0 init, atoi() again. Whydoyouwriteitallwithoutspaces?
>
> --
> vda
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: busybox-1.8.1_udhcp.patch_v2
Type: application/octet-stream
Size: 9434 bytes
Desc: not available
Url : http://busybox.net/lists/busybox/attachments/20071227/7b42aa77/attachment-0002.obj
-------------- next part --------------
A non-text attachment was scrubbed...
Name: config
Type: application/octet-stream
Size: 20898 bytes
Desc: not available
Url : http://busybox.net/lists/busybox/attachments/20071227/7b42aa77/attachment-0003.obj
More information about the busybox
mailing list