Re: getent: use more appropiate types/limits around strtonum()

2018-09-26 Thread Klemens Nanni 
On Wed, Sep 26, 2018 at 06:48:07AM -0600, Todd C. Miller wrote:
> One comment inline, otherwise OK millert@

> > @@ -397,6 +397,9 @@ static int
> >  services(int argc, char *argv[])
> >  {
> > struct servent  *se;
> > +   const char  *err;
> > +   char*proto;
> > +   int serv;
> 
> This should be in_port_t, not int.  I would also call the variable
> port instead of serv but the type is what is important.
Missed that one, thanks.

I'll commit it with `in_port_t port' later this evening unless someone
objects.

Re: getent: use more appropiate types/limits around strtonum()

2018-09-26 Thread Todd C. Miller 
On Wed, 26 Sep 2018 00:44:18 +0200, Klemens Nanni wrote:

> Replace `long long id' with appropiate types and names, use smaller
> limits where applicable and move variable declarations up out of loops.
>
> This makes the code clearer and a tad simpler while staying consistent
> across databases.

One comment inline, otherwise OK millert@

 - todd

> Index: getent.c
> ===
> RCS file: /cvs/src/usr.bin/getent/getent.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 getent.c
> --- getent.c  25 Sep 2018 19:51:39 -  1.18
> +++ getent.c  25 Sep 2018 22:21:22 -
> @@ -190,8 +190,10 @@ ethers(int argc, char *argv[])
>  static int
>  group(int argc, char *argv[])
>  {
> - int i, rv = RV_OK;
>   struct group*gr;
> + const char  *err;
> + gid_t   gid;
> + int i, rv = RV_OK;
>  
>   setgroupent(1);
>   if (argc == 2) {
> @@ -199,11 +201,9 @@ group(int argc, char *argv[])
>   GROUPPRINT;
>   } else {
>   for (i = 2; i < argc; i++) {
> - const char  *err;
> - long long id = strtonum(argv[i], 0, UINT_MAX, );
> -
> + gid = strtonum(argv[i], 0, GID_MAX, );
>   if (!err)
> - gr = getgrgid((gid_t)id);
> + gr = getgrgid(gid);
>   else
>   gr = getgrnam(argv[i]);
>   if (gr != NULL)
> @@ -291,8 +291,10 @@ hosts(int argc, char *argv[])
>  static int
>  passwd(int argc, char *argv[])
>  {
> - int i, rv = RV_OK;
>   struct passwd   *pw;
> + const char  *err;
> + uid_t   uid;
> + int i, rv = RV_OK;
>  
>   setpassent(1);
>   if (argc == 2) {
> @@ -300,11 +302,9 @@ passwd(int argc, char *argv[])
>   PASSWDPRINT;
>   } else {
>   for (i = 2; i < argc; i++) {
> - const char  *err;
> - long long id = strtonum(argv[i], 0, UINT_MAX, );
> -
> + uid = strtonum(argv[i], 0, UID_MAX, );
>   if (!err)
> - pw = getpwuid((uid_t)id);
> + pw = getpwuid(uid);
>   else
>   pw = getpwnam(argv[i]);
>   if (pw != NULL)
> @@ -327,6 +327,8 @@ static int
>  protocols(int argc, char *argv[])
>  {
>   struct protoent *pe;
> + const char  *err;
> + int proto;
>   int i, rv = RV_OK;
>  
>   setprotoent(1);
> @@ -335,11 +337,9 @@ protocols(int argc, char *argv[])
>   PROTOCOLSPRINT;
>   } else {
>   for (i = 2; i < argc; i++) {
> - const char  *err;
> - long long id = strtonum(argv[i], 0, UINT_MAX, );
> -
> + proto = strtonum(argv[i], 0, INT_MAX, );
>   if (!err)
> - pe = getprotobynumber((int)id);
> + pe = getprotobynumber(proto);
>   else
>   pe = getprotobyname(argv[i]);
>   if (pe != NULL)
> @@ -362,6 +362,8 @@ static int
>  rpc(int argc, char *argv[])
>  {
>   struct rpcent   *re;
> + const char  *err;
> + int rpc;
>   int i, rv = RV_OK;
>  
>   setrpcent(1);
> @@ -370,11 +372,9 @@ rpc(int argc, char *argv[])
>   RPCPRINT;
>   } else {
>   for (i = 2; i < argc; i++) {
> - const char  *err;
> - long long id = strtonum(argv[i], 0, UINT_MAX, );
> -
> + rpc = strtonum(argv[i], 0, INT_MAX, );
>   if (!err)
> - re = getrpcbynumber((int)id);
> + re = getrpcbynumber(rpc);
>   else
>   re = getrpcbyname(argv[i]);
>   if (re != NULL)
> @@ -397,6 +397,9 @@ static int
>  services(int argc, char *argv[])
>  {
>   struct servent  *se;
> + const char  *err;
> + char*proto;
> + int serv;

This should be in_port_t, not int.  I would also call the variable
port instead of serv but the type is what is important.

>   int i, rv = RV_OK;
>  
>   setservent(1);
> @@ -405,15 +408,11 @@ services(int argc, char *argv[])
>   SERVICESPRINT;
>   } else {
>   for (i = 2; i < argc; i++) {
> - const char  *err;
> - long long   id;
> - char *proto = strchr(argv[i], '/');
> -
> - if (proto != NULL)
> + if ((proto =

getent: use more appropiate types/limits around strtonum()

2018-09-25 Thread Klemens Nanni 
Replace `long long id' with appropiate types and names, use smaller
limits where applicable and move variable declarations up out of loops.

This makes the code clearer and a tad simpler while staying consistent
across databases.

Feedback? OK?

Index: getent.c
===
RCS file: /cvs/src/usr.bin/getent/getent.c,v
retrieving revision 1.18
diff -u -p -r1.18 getent.c
--- getent.c25 Sep 2018 19:51:39 -  1.18
+++ getent.c25 Sep 2018 22:21:22 -
@@ -190,8 +190,10 @@ ethers(int argc, char *argv[])
 static int
 group(int argc, char *argv[])
 {
-   int i, rv = RV_OK;
struct group*gr;
+   const char  *err;
+   gid_t   gid;
+   int i, rv = RV_OK;
 
setgroupent(1);
if (argc == 2) {
@@ -199,11 +201,9 @@ group(int argc, char *argv[])
GROUPPRINT;
} else {
for (i = 2; i < argc; i++) {
-   const char  *err;
-   long long id = strtonum(argv[i], 0, UINT_MAX, );
-
+   gid = strtonum(argv[i], 0, GID_MAX, );
if (!err)
-   gr = getgrgid((gid_t)id);
+   gr = getgrgid(gid);
else
gr = getgrnam(argv[i]);
if (gr != NULL)
@@ -291,8 +291,10 @@ hosts(int argc, char *argv[])
 static int
 passwd(int argc, char *argv[])
 {
-   int i, rv = RV_OK;
struct passwd   *pw;
+   const char  *err;
+   uid_t   uid;
+   int i, rv = RV_OK;
 
setpassent(1);
if (argc == 2) {
@@ -300,11 +302,9 @@ passwd(int argc, char *argv[])
PASSWDPRINT;
} else {
for (i = 2; i < argc; i++) {
-   const char  *err;
-   long long id = strtonum(argv[i], 0, UINT_MAX, );
-
+   uid = strtonum(argv[i], 0, UID_MAX, );
if (!err)
-   pw = getpwuid((uid_t)id);
+   pw = getpwuid(uid);
else
pw = getpwnam(argv[i]);
if (pw != NULL)
@@ -327,6 +327,8 @@ static int
 protocols(int argc, char *argv[])
 {
struct protoent *pe;
+   const char  *err;
+   int proto;
int i, rv = RV_OK;
 
setprotoent(1);
@@ -335,11 +337,9 @@ protocols(int argc, char *argv[])
PROTOCOLSPRINT;
} else {
for (i = 2; i < argc; i++) {
-   const char  *err;
-   long long id = strtonum(argv[i], 0, UINT_MAX, );
-
+   proto = strtonum(argv[i], 0, INT_MAX, );
if (!err)
-   pe = getprotobynumber((int)id);
+   pe = getprotobynumber(proto);
else
pe = getprotobyname(argv[i]);
if (pe != NULL)
@@ -362,6 +362,8 @@ static int
 rpc(int argc, char *argv[])
 {
struct rpcent   *re;
+   const char  *err;
+   int rpc;
int i, rv = RV_OK;
 
setrpcent(1);
@@ -370,11 +372,9 @@ rpc(int argc, char *argv[])
RPCPRINT;
} else {
for (i = 2; i < argc; i++) {
-   const char  *err;
-   long long id = strtonum(argv[i], 0, UINT_MAX, );
-
+   rpc = strtonum(argv[i], 0, INT_MAX, );
if (!err)
-   re = getrpcbynumber((int)id);
+   re = getrpcbynumber(rpc);
else
re = getrpcbyname(argv[i]);
if (re != NULL)
@@ -397,6 +397,9 @@ static int
 services(int argc, char *argv[])
 {
struct servent  *se;
+   const char  *err;
+   char*proto;
+   int serv;
int i, rv = RV_OK;
 
setservent(1);
@@ -405,15 +408,11 @@ services(int argc, char *argv[])
SERVICESPRINT;
} else {
for (i = 2; i < argc; i++) {
-   const char  *err;
-   long long   id;
-   char *proto = strchr(argv[i], '/');
-
-   if (proto != NULL)
+   if ((proto = strchr(argv[i], '/')) != NULL)
*proto++ = '\0';
-   id = strtonum(argv[i], 0, UINT_MAX, );
+   serv = strtonum(argv[i], 0, IPPORT_HILASTAUTO, );
if (!err)
-   se =