On 2010-06-15 00:08, Max Laier wrote:
I'm not sure about the intention behind the len assignements in libugidfw -
might be just a leftover - but if the idea is to teach a model that we
generally care about the return value of snprintf(), a void cast might be
the
more protable solution.
These specific snprintf() calls all occur just before returning an
error, so checking the return value is quite useless (unless one wanted
to output some sort of overflow warning right there).
Moreover, all calls to snprintf() in lib/libugidfw/ugidfw.c that do
check the return value are incorrect in two ways:
- The return value is stored in a size_t, while snprintf() returns an
int. Thus all the checks ret 0 become bogus.
- The idiom used everywhere is:
len = snprintf(cur, left, ...);
if (len 0 || len left)
goto truncated;
which is wrong; the second check should be len = left instead.
Please review the attached patch which fixes those problems too.
Index: lib/libugidfw/ugidfw.c
===
--- lib/libugidfw/ugidfw.c (revision 209192)
+++ lib/libugidfw/ugidfw.c (working copy)
@@ -63,22 +63,22 @@
struct passwd *pwd;
struct statfs *mntbuf;
char *cur, type[sizeof(rule-mbr_object.mbo_type) * CHAR_BIT + 1];
- size_t left, len;
- int anymode, unknownmode, truncated, numfs, i, notdone;
+ size_t left;
+ int len, anymode, unknownmode, truncated, numfs, i, notdone;
cur = buf;
left = buflen;
truncated = 0;
len = snprintf(cur, left, subject );
- if (len 0 || len left)
+ if (len 0 || len = left)
goto truncated;
left -= len;
cur += len;
if (rule-mbr_subject.mbs_flags) {
if (rule-mbr_subject.mbs_neg == MBS_ALL_FLAGS) {
len = snprintf(cur, left, not );
- if (len 0 || len left)
+ if (len 0 || len = left)
goto truncated;
left -= len;
cur += len;
@@ -89,7 +89,7 @@
if (!notdone (rule-mbr_subject.mbs_neg MBO_UID_DEFINED)) {
len = snprintf(cur, left, ! );
- if (len 0 || len left)
+ if (len 0 || len = left)
goto truncated;
left -= len;
cur += len;
@@ -99,14 +99,14 @@
if (pwd != NULL) {
len = snprintf(cur, left, uid %s,
pwd-pw_name);
- if (len 0 || len left)
+ if (len 0 || len = left)
goto truncated;
left -= len;
cur += len;
} else {
len = snprintf(cur, left, uid %u,
rule-mbr_subject.mbs_uid_min);
- if (len 0 || len left)
+ if (len 0 || len = left)
goto truncated;
left -= len;
cur += len;
@@ -117,21 +117,21 @@
if (pwd != NULL) {
len = snprintf(cur, left, :%s ,
pwd-pw_name);
- if (len 0 || len left)
+ if (len 0 || len = left)
goto truncated;
left -= len;
cur += len;
} else {
len = snprintf(cur, left, :%u ,
rule-mbr_subject.mbs_uid_max);
- if (len 0 || len left)
+ if (len 0 || len = left)
goto truncated;
left -= len;
cur += len;
}
} else {
len = snprintf(cur, left, );
- if (len 0 || len left)
+ if (len 0 || len = left)
goto truncated;
left -= len;
cur += len;
@@ -139,7 +139,7 @@
}
if (!notdone (rule-mbr_subject.mbs_neg MBO_GID_DEFINED)) {
len = snprintf(cur, left, ! );
- if (len 0 || len left)
+
