[hackers] [PATCH][sdhcp] Coding style and minor fixes
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
_AT_@ -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
_AT_@ -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
_AT_@ -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
_AT_@ -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
_AT_@ -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];
_AT_@ -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,
_AT_@ -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";
_AT_@ -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}
_AT_@ -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 *
_AT_@ -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;
}
_AT_@ -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;
}
_AT_@ -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;
}
_AT_@ -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);
_AT_@ -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);
}
_AT_@ -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;
_AT_@ -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));
_AT_@ -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;
_AT_@ -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;
}
_AT_@ -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);
_AT_@ -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;
_AT_@ -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()) {
_AT_@ -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;
}
_AT_@ -403,7 +435,7 @@ Renewing:
goto Bound;
case DHCPnak:
goto Init;
- case Timeout:
+ case TIMEOUT:
dhcpsend(DHCPrequest, Broadcast);
goto Rebinding;
}
_AT_@ -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
_AT_@ -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
_AT_@ -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);
_AT_@ -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
_AT_@ -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 */
}
Received on Mon Sep 26 2016 - 10:26:41 CEST
This archive was generated by hypermail 2.3.0
: Mon Sep 26 2016 - 10:36:14 CEST