On Sat, 10 Oct 2015, Michael McConville wrote:
> FWIW, this is a perfect use case for Coccinelle. Below is what I dredged
> up in src/usr.sbin (diff not yet carefully audited, but apparently
> sane).
The ypserv chunks show your Coccinelle script could use an enhancement...
> --- ypserv/ypserv/acl.c
> +++ /tmp/cocci-output-2270-df68c1-acl.c
> @@ -161,7 +161,7 @@ acl_init(char *file)
> k = p; /* save start of verb */
> i = 0;
> while (*p != '\0' &&
> - !isspace((*p = tolower(*p)))) {
> + !isspace((unsigned char)(*p = tolower(*p)))) {
tolower() and toupper() follow the same rules as is*(), so the above is
still wrong: if a cast is needed for isspace() then it's also needed for
tolower(). Can you enhance your Coccinelle script to apply the same rules
to them?
This is also a case where changing the type of the variable is the cleaner
solution, IMO.
Finally, this code also has an incorrect/unnecessary use of '& of array'.
Given these declarations:
char data_line[1024], *p;
this line is bogus:
p = (char *) &data_line;
It should be either:
p = data_line;
or
p = &data_line[0];
(With the type change to 'p', however, the cast will still be needed in
the revised diff below)
Does the diff below pass your Coccinelle script?
oks?
Philip Guenther
Index: usr.sbin/ypserv/ypserv/acl.c
===================================================================
RCS file: /cvs/src/usr.sbin/ypserv/ypserv/acl.c,v
retrieving revision 1.15
diff -u -p -r1.15 acl.c
--- usr.sbin/ypserv/ypserv/acl.c 4 Dec 2013 02:18:05 -0000 1.15
+++ usr.sbin/ypserv/ypserv/acl.c 11 Oct 2015 02:46:11 -0000
@@ -133,7 +133,8 @@ acl_add_host(int allow, struct in_addr *
int
acl_init(char *file)
{
- char data_line[1024], *p, *k;
+ char data_line[1024], *k;
+ unsigned char *p;
int line_no = 0, len, i, state;
int allow = TRUE, error_cnt = 0;
struct in_addr addr, mask, *host_addr;
@@ -151,7 +152,7 @@ acl_init(char *file)
len = strlen(data_line);
if (len == 0)
continue;
- p = (char *) &data_line;
+ p = (unsigned char *)data_line;
/* State 1: Initial State */
@@ -420,7 +421,8 @@ acl_init(char *file)
int
acl_securenet(char *file)
{
- char data_line[1024], *p, *k;
+ char data_line[1024], *k;
+ unsigned char *p;
int line_no = 0, len, i, allow = TRUE, state;
int error_cnt = 0;
struct in_addr addr, mask;
@@ -442,7 +444,7 @@ acl_securenet(char *file)
len = strlen(data_line);
if (len == 0)
continue;
- p = (char *) &data_line;
+ p = (unsigned char *)data_line;
/* State 1: Initial State */
state = ACLS_INIT;
