Hi list,
I'm new to suckless. Here are some small patches for sdhcp.
None of these changes have big impact of the functionallity, AFAIK.
Suggestions on how to test this properly? I
am not a DHCP wizard.
Minor changes
--------------
* Fixing enum WTF "multiple-entries-with-the-same-value" insanity.
* Making 'Timeout' a macro instead of enum. Change case to TIMEOUT.
* Correcting snprintf() in setdns()
* Sane buffer size in setdns().
* Sane buffer size in acceptlease().
* Removed silly "Congrats! You should be on the 'net.", now report IP addr
instead, because that suck less.
* Adding silent mode option, -s, which now give no output to stdout on
successful IP address reception.
This new functionality might suck, you tell me.
* Updated Makefile/config.mk to link statically.
* Verbose usage(). This change might acctually suck.
* Updated man page
* Updated TODO
Changes to comply to coding style
----------------------------------
* Removed naming of Bootp struct, to comply with given coding style.
* Make if (XYZ < 0) {}, instead of if (XYZ == -1) {}, to comply with given
coding style.
* Add selfexplainatory macros, DHCP_CLIENT_PORT and DHCP_CLIENT_PORT, instead
of magical numbers.
* Adding {} after all if-statements and while-statements.
* Adding () to all sizeof operators.
* Indenting strlcpy.c with tabs
Comments and guidance on this would be appreciated.
Regards,
Ramsey
diff --git a/TODO b/TODO
index be4d6cf..88910cb 100644
--- a/TODO
+++ b/TODO
@@ -4,6 +4,8 @@ TODO:
probably skip in run() Init: etc stages.
[ ] replace unsigned char ip[4] and so on from function declarations.
[?] ipv6 support ?
+[ ] Remove gotos
+[ ] Add switch -r, Release ip binding
Changed (for now):
- cleanup
diff --git a/config.mk b/config.mk
index b3ac9aa..dffa813 100644
--- a/config.mk
+++ b/config.mk
@@ -9,4 +9,5 @@ CC = cc
LD = $(CC)
CPPFLAGS = -D_BSD_SOURCE
CFLAGS = -Wall -Wextra -pedantic -std=c99 $(CPPFLAGS)
-LDFLAGS = -s
+LDFLAGS = -s -static
+
diff --git a/sdhcp.1 b/sdhcp.1
index ba964a1..02846a7 100644
--- a/sdhcp.1
+++ b/sdhcp.1
@@ -10,6 +10,7 @@
.Op Fl e Ar program
.Op Fl f
.Op Fl i
+.Op Fl s
.Op Ar interface
.Op Ar client-id
.Sh DESCRIPTION
@@ -30,9 +31,11 @@ Variables will be set, see VARIABLES.
run in foreground.
.It Fl i
don't change interface information such as an IP address.
+.It Fl s
+silent mode, dont output to stdout on successful IP retrieval.
.El
.Sh VARIABLES
-The following variables are set:
+The following variables are set, if the option '-e program' is given:
.Bl -tag -width Ds
.It Ev SERVER
DHCP IP.
diff --git a/sdhcp.c b/sdhcp.c
index 5f829ad..426cbba 100644
--- a/sdhcp.c
+++ b/sdhcp.c
@@ -18,7 +18,7 @@
#include "arg.h"
#include "util.h"
-typedef struct bootp {
+typedef struct {
unsigned char op [1];
unsigned char htype [1];
unsigned char hlen [1];
@@ -46,13 +46,19 @@ enum {
DHCPnak,
DHCPrelease,
DHCPinform,
- Timeout = 200,
+};
+enum {
Bootrequest = 1,
Bootreply = 2,
+};
+
+enum {
/* bootp flags */
Fbroadcast = 1 << 15,
+};
+enum {
OBpad = 0,
OBmask = 1,
OBrouter = 3,
@@ -76,13 +82,20 @@ enum {
OBend = 255,
};
-enum { Broadcast, Unicast};
+enum {
+ Broadcast,
+ Unicast
+};
+
+#define TIMEOUT 200
+#define DHCP_SERVER_PORT 67
+#define DHCP_CLIENT_PORT 68
Bootp bp;
unsigned char magic[] = {99, 130, 83, 99};
/* conf */
-static unsigned char xid[sizeof bp.xid];
+static unsigned char xid[sizeof(bp.xid)];
static unsigned char hwaddr[16];
static time_t starttime;
static char *ifname = "eth0";
@@ -100,6 +113,7 @@ static unsigned long t1;
static int dflag = 1; /* change DNS in /etc/resolv.conf ? */
static int iflag = 1; /* set IP ? */
static int fflag = 0; /* run in foreground */
+static int silent_mode_flag = 0; /* 0 = output to stdout on IP retrival
success, 1 = no output */
#define IP(a,b,c,d) (unsigned char[4]){a,b,c,d}
@@ -108,8 +122,9 @@ hnput(unsigned char *dst, unsigned long long src, size_t n)
{
unsigned int i;
- for (i = 0; n--; i++)
+ for (i = 0; n--; i++) {
dst[i] = (src >> (n * 8)) & 0xff;
+ }
}
static struct sockaddr *
@@ -119,7 +134,7 @@ iptoaddr(struct sockaddr *ifaddr, unsigned char ip[4], int
port)
in->sin_family = AF_INET;
in->sin_port = htons(port);
- memcpy(&(in->sin_addr), ip, sizeof in->sin_addr);
+ memcpy(&(in->sin_addr), ip, sizeof(in->sin_addr));
return ifaddr;
}
@@ -129,12 +144,13 @@ static ssize_t
udpsend(unsigned char ip[4], int fd, void *data, size_t n)
{
struct sockaddr addr;
- socklen_t addrlen = sizeof addr;
+ socklen_t addrlen = sizeof(addr);
ssize_t sent;
- iptoaddr(&addr, ip, 67); /* bootp server */
- if ((sent = sendto(fd, data, n, 0, &addr, addrlen)) == -1)
+ iptoaddr(&addr, ip, DHCP_SERVER_PORT); /* bootp server */
+ if ((sent = sendto(fd, data, n, 0, &addr, addrlen)) < 0) {
eprintf("sendto:");
+ }
return sent;
}
@@ -144,12 +160,13 @@ static ssize_t
udprecv(unsigned char ip[4], int fd, void *data, size_t n)
{
struct sockaddr addr;
- socklen_t addrlen = sizeof addr;
+ socklen_t addrlen = sizeof(addr);
ssize_t r;
- iptoaddr(&addr, ip, 68); /* bootp client */
- if ((r = recvfrom(fd, data, n, 0, &addr, &addrlen)) == -1)
+ iptoaddr(&addr, ip, DHCP_CLIENT_PORT); /* bootp client */
+ if ((r = recvfrom(fd, data, n, 0, &addr, &addrlen)) < 0) {
eprintf("recvfrom:");
+ }
return r;
}
@@ -166,8 +183,9 @@ setip(unsigned char ip[4], unsigned char mask[4], unsigned
char gateway[4])
strlcpy(ifreq.ifr_name, ifname, IF_NAMESIZE);
iptoaddr(&(ifreq.ifr_addr), ip, 0);
- if ((fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_IP)) == -1)
+ if ((fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_IP)) < 0) {
eprintf("can't set ip, socket:");
+ }
ioctl(fd, SIOCSIFADDR, &ifreq);
iptoaddr(&(ifreq.ifr_netmask), mask, 0);
ioctl(fd, SIOCSIFNETMASK, &ifreq);
@@ -189,27 +207,31 @@ cat(int dfd, char *src)
char buf[BUFSIZ];
int n, fd;
- if ((fd = open(src, O_RDONLY)) == -1)
+ if ((fd = open(src, O_RDONLY)) < 0) {
return; /* can't read, but don't error out */
- while ((n = read(fd, buf, sizeof buf)) > 0)
+ }
+ while ((n = read(fd, buf, sizeof(buf))) > 0) {
write(dfd, buf, n);
+ }
close(fd);
}
static void
setdns(unsigned char dns[4])
{
- char buf[128];
+ char buf[29]; /* 29 = sizeof("\nnameserver 123.123.123.123\n") */
+
int fd;
- if ((fd = creat("/etc/resolv.conf", 0644)) == -1) {
+ if ((fd = creat("/etc/resolv.conf", 0644)) < 0) {
weprintf("can't change /etc/resolv.conf:");
return;
}
cat(fd, "/etc/resolv.conf.head");
- if (snprintf(buf, sizeof(buf) - 1, "\nnameserver %d.%d.%d.%d\n",
- dns[0], dns[1], dns[2], dns[3]) > 0)
+ if (snprintf(buf, sizeof(buf), "\nnameserver %d.%d.%d.%d\n",
+ dns[0], dns[1], dns[2], dns[3]) > 0) {
write(fd, buf, strlen(buf));
+ }
cat(fd, "/etc/resolv.conf.tail");
close(fd);
}
@@ -218,18 +240,21 @@ static void
optget(Bootp *bp, void *data, int opt, int n)
{
unsigned char *p = bp->optdata;
- unsigned char *top = ((unsigned char *)bp) + sizeof *bp;
+ unsigned char *top = ((unsigned char *)bp) + sizeof(*bp);
int code, len;
while (p < top) {
code = *p++;
- if (code == OBpad)
+ if (code == OBpad) {
continue;
- if (code == OBend || p == top)
+ }
+ if (code == OBend || p == top) {
break;
+ }
len = *p++;
- if (len > top - p)
+ if (len > top - p) {
break;
+ }
if (code == opt) {
memcpy(data, p, MIN(len, n));
break;
@@ -263,15 +288,15 @@ dhcpsend(int type, int how)
{
unsigned char *ip, *p;
- memset(&bp, 0, sizeof bp);
+ memset(&bp, 0, sizeof(bp));
hnput(bp.op, Bootrequest, 1);
hnput(bp.htype, 1, 1);
hnput(bp.hlen, 6, 1);
- memcpy(bp.xid, xid, sizeof xid);
- hnput(bp.flags, Fbroadcast, sizeof bp.flags);
- hnput(bp.secs, time(NULL) - starttime, sizeof bp.secs);
- memcpy(bp.magic, magic, sizeof bp.magic);
- memcpy(bp.chaddr, hwaddr, sizeof bp.chaddr);
+ memcpy(bp.xid, xid, sizeof(xid));
+ hnput(bp.flags, Fbroadcast, sizeof(bp.flags));
+ hnput(bp.secs, time(NULL) - starttime, sizeof(bp.secs));
+ memcpy(bp.magic, magic, sizeof(bp.magic));
+ memcpy(bp.chaddr, hwaddr, sizeof(bp.chaddr));
p = bp.optdata;
p = hnoptput(p, ODtype, type, 1);
p = optput(p, ODclientid, cid, sizeof(cid));
@@ -281,14 +306,14 @@ dhcpsend(int type, int how)
break;
case DHCPrequest:
/* memcpy(bp.ciaddr, client, sizeof bp.ciaddr); */
- p = hnoptput(p, ODlease, t1, sizeof t1);
- p = optput(p, ODipaddr, client, sizeof client);
- p = optput(p, ODserverid, server, sizeof server);
+ p = hnoptput(p, ODlease, t1, sizeof(t1));
+ p = optput(p, ODipaddr, client, sizeof(client));
+ p = optput(p, ODserverid, server, sizeof(server));
break;
case DHCPrelease:
- memcpy(bp.ciaddr, client, sizeof client);
- p = optput(p, ODipaddr, client, sizeof client);
- p = optput(p, ODserverid, server, sizeof server);
+ memcpy(bp.ciaddr, client, sizeof(client));
+ p = optput(p, ODipaddr, client, sizeof(client));
+ p = optput(p, ODserverid, server, sizeof(server));
break;
}
*p++ = OBend;
@@ -307,15 +332,17 @@ dhcprecv(void)
pfd.fd = sock;
pfd.events = POLLIN;
- memset(&bp, 0, sizeof bp);
- if (poll(&pfd, 1, -1) == -1) {
- if (errno != EINTR)
+ memset(&bp, 0, sizeof(bp));
+ if (poll(&pfd, 1, -1) < 0) {
+ if (errno != EINTR) {
eprintf("poll:");
- else
- return Timeout;
+ }
+ else {
+ return TIMEOUT;
+ }
}
- udprecv(IP(255, 255, 255, 255), sock, &bp, sizeof bp);
- optget(&bp, &type, ODtype, sizeof type);
+ udprecv(IP(255, 255, 255, 255), sock, &bp, sizeof(bp));
+ optget(&bp, &type, ODtype, sizeof(type));
return type;
}
@@ -323,12 +350,14 @@ dhcprecv(void)
static void
acceptlease(void)
{
- char buf[128];
+ char buf[16]; /* 16 = sizeof("123.123.123.123") */
- if (iflag)
+ if (iflag) {
setip(client, mask, router);
- if (dflag)
+ }
+ if (dflag) {
setdns(dns);
+ }
if (*program) {
snprintf(buf, sizeof(buf), "%d.%d.%d.%d", server[0], server[1],
server[2], server[3]);
setenv("SERVER", buf, 1);
@@ -358,16 +387,16 @@ Selecting:
switch(dhcprecv()) {
case DHCPoffer:
alarm(0);
- memcpy(client, bp.yiaddr, sizeof client);
- optget(&bp, server, ODserverid, sizeof server);
- optget(&bp, mask, OBmask, sizeof mask);
- optget(&bp, router, OBrouter, sizeof router);
- optget(&bp, dns, OBdnsserver, sizeof dns);
- optget(&bp, &t1, ODlease, sizeof t1);
+ memcpy(client, bp.yiaddr, sizeof(client));
+ optget(&bp, server, ODserverid, sizeof(server));
+ optget(&bp, mask, OBmask, sizeof(mask));
+ optget(&bp, router, OBrouter, sizeof(router));
+ optget(&bp, dns, OBdnsserver, sizeof(dns));
+ optget(&bp, &t1, ODlease, sizeof(t1));
t1 = ntohl(t1);
dhcpsend(DHCPrequest, Broadcast);
goto Requesting;
- case Timeout:
+ case TIMEOUT:
goto Init;
default:
goto Selecting;
@@ -381,10 +410,13 @@ Requesting:
goto Bound;
}
Bound:
- fputs("Congrats! You should be on the 'net.\n", stdout);
+ if(!silent_mode_flag) {
+ fprintf(stdout, "Got %d.%d.%d.%d\n", client[0], client[1],
client[2], client[3]);
+ }
if (!fflag && !forked) {
- if (fork())
+ if (fork()) {
exit(0);
+ }
forked = 1;
}
switch (dhcprecv()) {
@@ -392,7 +424,7 @@ Bound:
case DHCPack:
case DHCPnak:
goto Bound; /* discard offer, ACK or NAK */
- case Timeout:
+ case TIMEOUT:
dhcpsend(DHCPrequest, Unicast);
goto Renewing;
}
@@ -403,7 +435,7 @@ Renewing:
goto Bound;
case DHCPnak:
goto Init;
- case Timeout:
+ case TIMEOUT:
dhcpsend(DHCPrequest, Broadcast);
goto Rebinding;
}
@@ -434,7 +466,7 @@ void cleanexit(int unused)
static void
usage(void)
{
- eprintf("usage: %s [-d] [-e program] [-f] [-i] [ifname] [clientid]\n",
argv0);
+ eprintf("usage: %s [-d(ont update DNS in resolve.conf)] [-e(xec) program]
[-f(oreground)] [-i(p not set)] [-s(ilent stdout)] [ifname] [clientid]\n", argv0);
}
int
@@ -458,40 +490,54 @@ main(int argc, char *argv[])
case 'i': /* don't set ip */
iflag = 0;
break;
+ case 's': /* Silent mode */
+ silent_mode_flag = 1;
+ break;
default:
usage();
break;
} ARGEND;
- if (argc)
+ if (argc) {
ifname = argv[0]; /* interface name */
- if (argc >= 2)
- strlcpy(cid, argv[1], sizeof(cid)); /* client-id */
+ }
+ if (argc >= 2) {
+ strlcpy((char *)cid, argv[1], sizeof(cid)); /* client-id */
+ }
memset(&ifreq, 0, sizeof(ifreq));
signal(SIGALRM, nop);
signal(SIGTERM, cleanexit);
- if ((sock = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
+ if ((sock = socket(AF_INET, SOCK_DGRAM, 0)) < 0) {
eprintf("socket:");
- if (setsockopt(sock, SOL_SOCKET, SO_BROADCAST, &bcast, sizeof bcast) ==
-1)
+ }
+ if (setsockopt(sock, SOL_SOCKET, SO_BROADCAST, &bcast, sizeof(bcast)) <
0) {
eprintf("setsockopt:");
+ }
strlcpy(ifreq.ifr_name, ifname, IF_NAMESIZE);
ioctl(sock, SIOCGIFINDEX, &ifreq);
- if (setsockopt(sock, SOL_SOCKET, SO_BINDTODEVICE, &ifreq, sizeof ifreq)
== -1)
+ if (setsockopt(sock, SOL_SOCKET, SO_BINDTODEVICE, &ifreq, sizeof(ifreq))
< 0) {
eprintf("setsockopt:");
- iptoaddr(&addr, IP(255, 255, 255, 255), 68);
- if (bind(sock, (void*)&addr, sizeof addr) != 0)
+ }
+ iptoaddr(&addr, IP(255, 255, 255, 255), DHCP_CLIENT_PORT);
+ if (bind(sock, (void*)&addr, sizeof(addr)) != 0) {
+ if (errno == EADDRINUSE) {
+ //Release addr?
+ }
eprintf("bind:");
+ }
ioctl(sock, SIOCGIFHWADDR, &ifreq);
- memcpy(hwaddr, ifreq.ifr_hwaddr.sa_data, sizeof
ifreq.ifr_hwaddr.sa_data);
- if (!cid[0])
+ memcpy(hwaddr, ifreq.ifr_hwaddr.sa_data,
sizeof(ifreq.ifr_hwaddr.sa_data));
+ if (!cid[0]) {
memcpy(cid, hwaddr, sizeof(cid));
+ }
- if ((rnd = open("/dev/urandom", O_RDONLY)) == -1)
+ if ((rnd = open("/dev/urandom", O_RDONLY)) < 0) {
eprintf("can't open /dev/urandom to generate unique transaction
identifier:");
- read(rnd, xid, sizeof xid);
+ }
+ read(rnd, xid, sizeof(xid));
close(rnd);
starttime = time(NULL);
diff --git a/util/eprintf.c b/util/eprintf.c
index 4d8f726..bd8cd6c 100644
--- a/util/eprintf.c
+++ b/util/eprintf.c
@@ -33,8 +33,9 @@ enprintf(int status, const char *fmt, ...)
void
venprintf(int status, const char *fmt, va_list ap)
{
- if (strncmp(fmt, "usage", strlen("usage")))
+ if (strncmp(fmt, "usage", strlen("usage"))) {
fprintf(stderr, "%s: ", argv0);
+ }
vfprintf(stderr, fmt, ap);
@@ -51,8 +52,9 @@ weprintf(const char *fmt, ...)
{
va_list ap;
- if (strncmp(fmt, "usage", strlen("usage")))
+ if (strncmp(fmt, "usage", strlen("usage"))) {
fprintf(stderr, "%s: ", argv0);
+ }
va_start(ap, fmt);
vfprintf(stderr, fmt, ap);
diff --git a/util/strlcpy.c b/util/strlcpy.c
index 62fb7f6..f4d42df 100644
--- a/util/strlcpy.c
+++ b/util/strlcpy.c
@@ -11,22 +11,23 @@
size_t
strlcpy(char *dst, const char *src, size_t siz)
{
- char *d = dst;
- const char *s = src;
- size_t n = siz;
- /* Copy as many bytes as will fit */
- if (n != 0) {
- while (--n != 0) {
- if ((*d++ = *s++) == '\0')
- break;
- }
- }
- /* Not enough room in dst, add NUL and traverse rest of src */
- if (n == 0) {
- if (siz != 0)
- *d = '\0'; /* NUL-terminate dst */
- while (*s++)
- ;
- }
- return(s - src - 1); /* count does not include NUL */
+ char *d = dst;
+ const char *s = src;
+ size_t n = siz;
+ /* Copy as many bytes as will fit */
+ if (n != 0) {
+ while (--n != 0) {
+ if ((*d++ = *s++) == '\0') {
+ break;
+ }
+ }
+ }
+ /* Not enough room in dst, add NUL and traverse rest of src */
+ if (n == 0) {
+ if (siz != 0) {
+ *d = '\0'; /* NUL-terminate dst */
+ }
+ while (*s++) {}
+ }
+ return(s - src - 1); /* count does not include NUL */
}
