MInor fixes and problems for malloced getpw/grxxx functions for bb
tito
farmatito at tiscali.it
Sun Jan 4 23:14:04 UTC 2015
Hi Denys,
while trying to fully understand the new pwd_grp code I've spotted
some minor problems that need to be fixed:
1) in function parse_common count should start at 1 as line numbers are off by one.
--- libpwdgrp/pwd_grp.c.original 2015-01-04 22:36:40.000000000 +0100
+++ libpwdgrp/pwd_grp.c 2015-01-04 22:40:17.904401220 +0100
@@ -189,7 +189,7 @@ static int tokenize(char *buffer, int ch
static char *parse_common(FILE *fp, struct passdb *db,
const char *key, int field_pos)
{
- int count = 0;
+ int count = 1;
char *buf;
while ((buf = xmalloc_fgetline(fp)) != NULL) {
2) in parse_common the use of key variable seems problematic to me:
if (!key) {
/* no key specified: sequential read, return a record */
break;
}
This variable string is passed on from the caller and eventually from the user
we have no guarantee that it will be non NULL when we don't want a sequential read.
If the calling app passes a bad pointer it will get a record returned.
I used field_pos set to -1 to signal the parser that I wanted a sequential read.
if (field_pos >= 0) {
if (*(s = nth_string(buf, field_pos))) {
if (key && *key) {
if (strcmp(key, s) == 0) {
/* record found */
break;
}
} /* else skip: caller passed bad key */
} /* else skip: field is empty */
} else { /* field_pos < 0 */
/* No field specified (-1): sequential read, return current record */
break;
}
3) in parse_common function
while ((buf = xmalloc_fgetline(fp)) != NULL) {
does not set errno to ENOENT on EOF, this makes segfault
code that works with glibc like e.g.:
while (1) {
ret = getpwent_r(&pwd, buffer, 256, &pw);
if (ret) {
if (ret != ENOENT) {
printf("getpwent_r: ret %s\n", strerror(ret));
}
break;
}
printf("%s (%d)\tHOME %s\tSHELL %s\n", pw->pw_name,
pw->pw_uid, pw->pw_dir, pw->pw_shell);
}
Man page for getpwent_r says:
RETURN VALUE
On success, these functions return 0 and *pwbufp is a pointer to the struct passwd. On
error, these functions return an error value and *pwbufp is NULL.
ERRORS
ENOENT No more entries.
ERANGE Insufficient buffer space supplied. Try again with larger buffer.
The fix with the actual code is not so easy because if you fix the getpwent_r case (return ENOENT on EOF)
you break the getpwnam_r/getspnam_r case (0 on USER not found).
4) in function parse_common:
while (p < S.tokenize_end)
if (*p++ == ',')
cnt++;
would be more readable with braces {}, but that is just my personal taste ;-)
Ciao,
Tito
More information about the busybox
mailing list