| Summary: | udhcpd: using preprocessor #if for WRITE_LEASES_EARLY | ||
|---|---|---|---|
| Product: | Busybox | Reporter: | Jiri J. <jirij.jabb> |
| Component: | Other | Assignee: | unassigned |
| Status: | RESOLVED WONTFIX | ||
| Severity: | enhancement | CC: | busybox-cvs |
| Priority: | P5 | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Host: | Target: | ||
| Build: | |||
Previous maintainer is heavily in favor for if(ENABLExxx). It does not look as bad as #if forest, and lets compiler check syntax inside if's.
I use both forms. I slightly prefer if(ENABLExxx) since if you have many #if ENABLExxx, it starts to be unmanageable. Look at networking/httpd.c, for example:
# grep '^# *if' httpd.c | wc -l
70
#if ENABLE_FEATURE_HTTPD_CGI
static const char request_HEAD[] ALIGN1 = "HEAD";
const char *prequest;
char *cookie = NULL;
char *content_type = NULL;
unsigned long length = 0;
#elif ENABLE_FEATURE_HTTPD_PROXY
#define prequest request_GET
unsigned long length = 0;
#endif
#if ENABLE_FEATURE_HTTPD_BASIC_AUTH
smallint authorized = -1;
#endif
smallint ip_allowed;
char http_major_version;
#if ENABLE_FEATURE_HTTPD_PROXY
char http_minor_version;
char *header_buf = header_buf; /* for gcc */
char *header_ptr = header_ptr;
Htaccess_Proxy *proxy_entry;
#endif
/* Allocation of iobuf is postponed until now
* (IOW, server process doesn't need to waste 8k) */
iobuf = xmalloc(IOBUF_SIZE);
rmt_ip = 0;
if (fromAddr->u.sa.sa_family == AF_INET) {
rmt_ip = ntohl(fromAddr->u.sin.sin_addr.s_addr);
}
#if ENABLE_FEATURE_IPV6
if (fromAddr->u.sa.sa_family == AF_INET6
&& fromAddr->u.sin6.sin6_addr.s6_addr32[0] == 0
&& fromAddr->u.sin6.sin6_addr.s6_addr32[1] == 0
&& ntohl(fromAddr->u.sin6.sin6_addr.s6_addr32[2]) == 0xffff)
rmt_ip = ntohl(fromAddr->u.sin6.sin6_addr.s6_addr32[3]);
#endif
O M G...
|
I've searched through the udhcpd sources (networking/udhcp/) and found only this used as if(...), all others are #if ... I'm not sure what style do you prefer, whether it's better to filter this on the preprocessor level or on the compiler level (using optimizations). I personally prefer preprocessor in this case, so diff --git a/networking/udhcp/serverpacket.c b/networking/udhcp/serverpacket.c index b48e415..3d5f181 100644 --- a/networking/udhcp/serverpacket.c +++ b/networking/udhcp/serverpacket.c @@ -254,10 +254,11 @@ int FAST_FUNC send_ACK(struct dhcp_packet *oldpacket, uint32_t yiaddr) p_host_name, p_host_name ? (unsigned char)p_host_name[OPT_LEN - OPT_DATA] : 0 ); - if (ENABLE_FEATURE_UDHCPD_WRITE_LEASES_EARLY) { + +#if ENABLE_FEATURE_UDHCPD_WRITE_LEASES_EARLY /* rewrite the file with leases at every new acceptance */ write_leases(); - } +#endif return 0; } (please pull [--rebase] from git://repo.or.cz/jirij.git, branch busybox_if) Further search revealed a lot of if(ENABLE_FEATURE_*) conditions, some of them with additional checks like if(ENABLE_FEATURE_SOMETHING && !ptr) - moving those to #if could make the code look more complicated, however for plain if(ENABLE_FEATURE_*), I'd suggest the ->preprocessor change. What's your opinion?