Hi,
gcc allows to mark variadic functions that expect a terminating NULL as
such and issues a warning if the caller doesn't pass the NULL. A similar
mechanism is available for printf-like functions. What about using this
for the few variadic functions in the rpmlib API? It would help to catch
some mistakes at compile-time rather than at run-time, e.g:
char *test1 = rpmExpand("%_sourcedir");
char *test2 = rpmExpand("%_topdir", " ");
$ gcc -Wall -I ./include -c test.c
test.c: In function 'main':
test.c:9: warning: not enough variable arguments to fit a sentinel
test.c:10: warning: missing sentinel in function call
Attached are two patches, the first one marks rpmExpand() and
rpmGetPath() with __attribute__((sentinel)) for gcc 4.0+ and rpmlog()
with __attribute__((format (printf, 2, 3))). The second patch silences
warnings in rpmlog() calls that now show up.
Comments?
Michal
diff -r 5165a84e8400 rpmio/rpmfileutil.h
--- a/rpmio/rpmfileutil.h Mon Feb 11 20:47:03 2008 +0200
+++ b/rpmio/rpmfileutil.h Wed Feb 13 15:55:03 2008 +0100
@@ -83,7 +83,12 @@ char * rpmGenPath (const char * urlroot,
* @param path macro(s) to expand (NULL terminates list)
* @return canonicalized path (malloc'ed)
*/
-char * rpmGetPath (const char * path, ...);
+char * rpmGetPath (const char * path, ...)
+#if defined(__GNUC__) && __GNUC__ >= 4
+ /* issue a warning if the list is not NULL-terminated */
+ __attribute__((sentinel))
+#endif
+ ;
/** \ingroup rpmfileutil
* Return URL path(s) from a (URL prefixed) pattern glob.
diff -r 5165a84e8400 rpmio/rpmlog.h
--- a/rpmio/rpmlog.h Mon Feb 11 20:47:03 2008 +0200
+++ b/rpmio/rpmlog.h Wed Feb 13 15:55:03 2008 +0100
@@ -220,7 +220,12 @@ int rpmlogSetMask (int mask);
/** \ingroup rpmlog
* Generate a log message using FMT string and option arguments.
*/
-void rpmlog (int code, const char *fmt, ...);
+void rpmlog (int code, const char *fmt, ...)
+#if defined(__GNUC__) && __GNUC__ >= 2
+ /* issue a warning if the format string doesn't match arguments */
+ __attribute__((format (printf, 2, 3)))
+#endif
+ ;
/** \ingroup rpmlog
* Return text of last rpmError() message.
diff -r 5165a84e8400 rpmio/rpmmacro.h
--- a/rpmio/rpmmacro.h Mon Feb 11 20:47:03 2008 +0200
+++ b/rpmio/rpmmacro.h Wed Feb 13 15:55:03 2008 +0100
@@ -124,7 +124,12 @@ void rpmFreeMacros (rpmMacroContext mc);
* @param arg macro(s) to expand (NULL terminates list)
* @return macro expansion (malloc'ed)
*/
-char * rpmExpand (const char * arg, ...);
+char * rpmExpand (const char * arg, ...)
+#if defined(__GNUC__) && __GNUC__ >= 4
+ /* issue a warning if the list is not NULL-terminated */
+ __attribute__((sentinel))
+#endif
+ ;
/** \ingroup rpmmacro
* Return macro expansion as a numeric value.
diff -r 5165a84e8400 lib/query.c
--- a/lib/query.c Mon Feb 11 20:47:03 2008 +0200
+++ b/lib/query.c Wed Feb 13 15:55:00 2008 +0100
@@ -641,12 +641,12 @@ int rpmQueryVerify(QVA_t qva, rpmts ts,
rpmlog(RPMLOG_NOTICE, _("invalid package number: %s\n"), arg);
return 1;
}
- rpmlog(RPMLOG_DEBUG, _("package record number: %u\n"), recOffset);
+ rpmlog(RPMLOG_DEBUG, _("package record number: %lu\n"), recOffset);
/* RPMDBI_PACKAGES */
qva->qva_mi = rpmtsInitIterator(ts, RPMDBI_PACKAGES, &recOffset, sizeof(recOffset));
if (qva->qva_mi == NULL) {
rpmlog(RPMLOG_NOTICE,
- _("record %u could not be read\n"), recOffset);
+ _("record %lu could not be read\n"), recOffset);
res = 1;
} else
res = rpmcliShowMatches(qva, ts);
diff -r 5165a84e8400 lib/rpminstall.c
--- a/lib/rpminstall.c Mon Feb 11 20:47:03 2008 +0200
+++ b/lib/rpminstall.c Wed Feb 13 15:55:00 2008 +0100
@@ -1006,7 +1006,7 @@ int rpmRollback(rpmts ts, struct rpmInst
tid = (time_t)thistid;
rpmlog(RPMLOG_NOTICE,
_("Rollback packages (+%d/-%d) to %-24.24s (0x%08x):\n"),
- numAdded, numRemoved, ctime(&tid), tid);
+ numAdded, numRemoved, ctime(&tid), thistid);
rc = rpmtsCheck(ts);
ps = rpmtsProblems(ts);
diff -r 5165a84e8400 lib/rpmts.c
--- a/lib/rpmts.c Mon Feb 11 20:47:03 2008 +0200
+++ b/lib/rpmts.c Wed Feb 13 15:55:00 2008 +0100
@@ -205,7 +205,7 @@ rpmdbMatchIterator rpmtsInitIterator(con
case '(':
/* XXX Fail if nested parens. */
if (level++ != 0) {
- rpmlog(RPMLOG_ERR, _("extra '(' in package label: %s\n"), keyp);
+ rpmlog(RPMLOG_ERR, _("extra '(' in package label: %s\n"), (const char*)keyp);
return NULL;
}
/* Parse explicit epoch. */
@@ -223,7 +223,7 @@ rpmdbMatchIterator rpmtsInitIterator(con
case ')':
/* XXX Fail if nested parens. */
if (--level != 0) {
- rpmlog(RPMLOG_ERR, _("missing '(' in package label: %s\n"), keyp);
+ rpmlog(RPMLOG_ERR, _("missing '(' in package label: %s\n"), (const char*)keyp);
return NULL;
}
/* Don't copy trailing ')' */
@@ -231,7 +231,7 @@ rpmdbMatchIterator rpmtsInitIterator(con
}
}
if (level) {
- rpmlog(RPMLOG_ERR, _("missing ')' in package label: %s\n"), keyp);
+ rpmlog(RPMLOG_ERR, _("missing ')' in package label: %s\n"), (const char*)keyp);
return NULL;
}
*t = '\0';
diff -r 5165a84e8400 lib/signature.c
--- a/lib/signature.c Mon Feb 11 20:47:03 2008 +0200
+++ b/lib/signature.c Wed Feb 13 15:55:00 2008 +0100
@@ -115,7 +115,7 @@ static inline rpmRC printSize(FD_t fd, s
return RPMRC_FAIL;
rpmlog(RPMLOG_DEBUG,
- _("Expected size: %12d = lead(%d)+sigs(%zd)+pad(%zd)+data(%zd)\n"),
+ _("Expected size: %12zd = lead(%d)+sigs(%zd)+pad(%zd)+data(%d)\n"),
RPMLEAD_SIZE+siglen+pad+datalen,
RPMLEAD_SIZE, siglen, pad, datalen);
rpmlog(RPMLOG_DEBUG,
@@ -431,7 +431,7 @@ static int makePGPSignature(const char *
}
*pktlenp = st.st_size;
- rpmlog(RPMLOG_DEBUG, _("PGP sig size: %d\n"), *pktlenp);
+ rpmlog(RPMLOG_DEBUG, _("PGP sig size: %zd\n"), *pktlenp);
*pktp = xmalloc(*pktlenp);
{ FD_t fd;
@@ -450,7 +450,7 @@ static int makePGPSignature(const char *
}
}
- rpmlog(RPMLOG_DEBUG, _("Got %d bytes of PGP sig\n"), *pktlenp);
+ rpmlog(RPMLOG_DEBUG, _("Got %zd bytes of PGP sig\n"), *pktlenp);
#ifdef NOTYET
/* Parse the signature, change signature tag as appropriate. */
@@ -542,7 +542,7 @@ static int makeGPGSignature(const char *
}
*pktlenp = st.st_size;
- rpmlog(RPMLOG_DEBUG, _("GPG sig size: %d\n"), *pktlenp);
+ rpmlog(RPMLOG_DEBUG, _("GPG sig size: %zd\n"), *pktlenp);
*pktp = xmalloc(*pktlenp);
{ FD_t fd;
@@ -561,7 +561,7 @@ static int makeGPGSignature(const char *
}
}
- rpmlog(RPMLOG_DEBUG, _("Got %d bytes of GPG sig\n"), *pktlenp);
+ rpmlog(RPMLOG_DEBUG, _("Got %zd bytes of GPG sig\n"), *pktlenp);
/* Parse the signature, change signature tag as appropriate. */
dig = pgpNewDig();
diff -r 5165a84e8400 lib/transaction.c
--- a/lib/transaction.c Mon Feb 11 20:47:03 2008 +0200
+++ b/lib/transaction.c Wed Feb 13 15:55:00 2008 +0100
@@ -976,7 +976,7 @@ static rpmRC _rpmtsRollback(rpmts rollba
/* Make sure the filename is still there. XXX: Can't happen */
key = rpmteKey(te);
if(key) {
- rpmlog(RPMLOG_NOTICE, _("\tRemoving %s:\n"), key);
+ rpmlog(RPMLOG_NOTICE, _("\tRemoving %s:\n"), (const char*)key);
(void) unlink(key); /* XXX: Should check for an error? */
}
break;
diff -r 5165a84e8400 rpmdb/rpmdb.c
--- a/rpmdb/rpmdb.c Mon Feb 11 20:47:03 2008 +0200
+++ b/rpmdb/rpmdb.c Wed Feb 13 15:55:00 2008 +0100
@@ -1144,7 +1144,7 @@ if (key->size == 0) key->size++; /* XXX
if (rc > 0) {
rpmlog(RPMLOG_ERR,
_("error(%d) getting \"%s\" records from %s index\n"),
- rc, key->data, rpmTagGetName(dbi->dbi_rpmtag));
+ rc, (char*)key->data, rpmTagGetName(dbi->dbi_rpmtag));
}
if (rc == 0)
@@ -1269,7 +1269,7 @@ key->size = strlen(name);
} else { /* error */
rpmlog(RPMLOG_ERR,
_("error(%d) getting \"%s\" records from %s index\n"),
- rc, key->data, rpmTagGetName(dbi->dbi_rpmtag));
+ rc, (char*)key->data, rpmTagGetName(dbi->dbi_rpmtag));
rc = -1;
}
@@ -1319,7 +1319,7 @@ key->size = strlen(name);
} else { /* error */
rpmlog(RPMLOG_ERR,
_("error(%d) getting \"%s\" records from %s index\n"),
- rc, key->data, rpmTagGetName(dbi->dbi_rpmtag));
+ rc, (char*)key->data, rpmTagGetName(dbi->dbi_rpmtag));
return RPMRC_FAIL;
}
@@ -2223,7 +2223,7 @@ static int rpmdbGrowIterator(rpmdbMatchI
if (rc != DB_NOTFOUND)
rpmlog(RPMLOG_ERR,
_("error(%d) getting \"%s\" records from %s index\n"),
- rc, key->data, rpmTagGetName(dbi->dbi_rpmtag));
+ rc, (char*)key->data, rpmTagGetName(dbi->dbi_rpmtag));
#ifdef SQLITE_HACK
xx = dbiCclose(dbi, dbcursor, 0);
dbcursor = NULL;
@@ -2342,7 +2342,7 @@ if (key->data && key->size == 0) key->si
if (rc > 0) {
rpmlog(RPMLOG_ERR,
_("error(%d) getting \"%s\" records from %s index\n"),
- rc, (key->data ? key->data : "???"), rpmTagGetName(dbi->dbi_rpmtag));
+ rc, (key->data ? (char *)key->data : "???"), rpmTagGetName(dbi->dbi_rpmtag));
}
/* Join keys need to be native endian internally. */
@@ -2632,7 +2632,7 @@ if (key->size == 0) key->size++; /* XXX
} else { /* error */
rpmlog(RPMLOG_ERR,
_("error(%d) setting \"%s\" records from %s index\n"),
- rc, key->data, rpmTagGetName(dbi->dbi_rpmtag));
+ rc, (char*)key->data, rpmTagGetName(dbi->dbi_rpmtag));
ret += 1;
continue;
}
@@ -2651,7 +2651,7 @@ if (key->size == 0) key->size++; /* XXX
if (rc) {
rpmlog(RPMLOG_ERR,
_("error(%d) storing record \"%s\" into %s\n"),
- rc, key->data, rpmTagGetName(dbi->dbi_rpmtag));
+ rc, (char*)key->data, rpmTagGetName(dbi->dbi_rpmtag));
ret += 1;
}
data->data = _free(data->data);
@@ -2661,7 +2661,7 @@ if (key->size == 0) key->size++; /* XXX
if (rc) {
rpmlog(RPMLOG_ERR,
_("error(%d) removing record \"%s\" from %s\n"),
- rc, key->data, rpmTagGetName(dbi->dbi_rpmtag));
+ rc, (char*)key->data, rpmTagGetName(dbi->dbi_rpmtag));
ret += 1;
}
}
@@ -3037,7 +3037,7 @@ if (key->size == 0) key->size++; /* XXX
} else if (rc != DB_NOTFOUND) { /* error */
rpmlog(RPMLOG_ERR,
_("error(%d) getting \"%s\" records from %s index\n"),
- rc, key->data, rpmTagGetName(dbi->dbi_rpmtag));
+ rc, (char*)key->data, rpmTagGetName(dbi->dbi_rpmtag));
ret += 1;
continue;
}
@@ -3053,7 +3053,7 @@ if (key->size == 0) key->size++; /* XXX
if (rc) {
rpmlog(RPMLOG_ERR,
_("error(%d) storing record %s into %s\n"),
- rc, key->data, rpmTagGetName(dbi->dbi_rpmtag));
+ rc, (char*)key->data, rpmTagGetName(dbi->dbi_rpmtag));
ret += 1;
}
data->data = _free(data->data);
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
https://lists.rpm.org/mailman/listinfo/rpm-maint
