Add printf format checks to afs_com_err()

Add gcc printf format checks to the afs_com_err() functions

Deal with the fallout, in particular change callers which pass
an empty format string to pass NULL instead - the com_err functions
already permit this alternate use.

There's a couple of real bugs here - in one case, we attempt to
print a NULL pointer, rather than a security index, and in the other
we supply a NULL format string, rather than the string we meant to
print.

Change-Id: Icd48f92a4447d4af3dba9a4caa2ff73c1657ad47
Reviewed-on: http://gerrit.openafs.org/794
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>

diff --git a/src/aklog/klog.c b/src/aklog/klog.c
index 0647c68..8120c0f 100644 (file)
--- a/src/aklog/klog.c
+++ b/src/aklog/klog.c
@@ -404,7 +404,7 @@ CommandProc(struct cmd_syndesc *as, void *arock)
     /* initialize_rx_error_table(); */
     if (!(tdir = afsconf_Open(AFSDIR_CLIENT_ETC_DIRPATH))) {
        afs_com_err(rn, 0, "can't get afs configuration (afsconf_Open(%s))",
-           rn, AFSDIR_CLIENT_ETC_DIRPATH);
+           AFSDIR_CLIENT_ETC_DIRPATH);
        KLOGEXIT(1);
     }
 
@@ -450,10 +450,10 @@ CommandProc(struct cmd_syndesc *as, void *arock)
 
     if (as->parms[aKRBREALM].items) {
        code = krb5_set_default_realm(k5context,
-               (const char *) as->parms[aKRBREALM].items);
+               as->parms[aKRBREALM].items->data);
        if (code) {
            afs_com_err(rn, code, "Can't make <%s> the default realm",
-               as->parms[aKRBREALM].items);
+               as->parms[aKRBREALM].items->data);
            KLOGEXIT(code);
        }
     }
@@ -623,7 +623,7 @@ CommandProc(struct cmd_syndesc *as, void *arock)
            break;
        Failed:
            if (code)
-               afs_com_err(rn, code, what);
+               afs_com_err(rn, code, "%s", what);
            if (writeTicketFile) {
                if (cc) {
                    krb5_cc_close(k5context, cc);
@@ -712,7 +712,7 @@ CommandProc(struct cmd_syndesc *as, void *arock)
            k5_to_k4_name(k5context, afscred->client, aclient);
            code = whoami(atoken, cellconfig, aclient, &viceid);
            if (code) {
-               afs_com_err(rn, code, "Can't get your viceid", cellconfig->name);
+               afs_com_err(rn, code, "Can't get your viceid for cell %s", cellconfig->name);
                *aclient->name = 0;
            } else
                snprintf(aclient->name, MAXKTCNAMELEN-1, "AFS ID %d", viceid);

diff --git a/src/bucoord/commands.c b/src/bucoord/commands.c
index d19803f..1cfdcc7 100644 (file)
--- a/src/bucoord/commands.c
+++ b/src/bucoord/commands.c
@@ -120,7 +120,7 @@ getSPEntries(afs_uint32 server, afs_int32 partition,
     if (!(*ss)) {
        *ss = (struct serversort *)malloc(sizeof(struct serversort));
        if (!(*ss)) {
-           afs_com_err(whoami, BC_NOMEM, "");
+           afs_com_err(whoami, BC_NOMEM, NULL);
            *ss = 0;
            return (BC_NOMEM);
        }
@@ -141,7 +141,7 @@ getSPEntries(afs_uint32 server, afs_int32 partition,
     if (!(*ps)) {
        *ps = (struct partitionsort *)malloc(sizeof(struct partitionsort));
        if (!(*ps)) {
-           afs_com_err(whoami, BC_NOMEM, "");
+           afs_com_err(whoami, BC_NOMEM, NULL);
            free(*ss);
            *ps = 0;
            *ss = 0;
@@ -286,7 +286,7 @@ EvalVolumeSet2(struct bc_config *aconfig,
                                 entries[e].serverPartition[ei], &servers,
                                 &ss, &ps);
                if (tcode) {
-                   afs_com_err(whoami, tcode, "");
+                   afs_com_err(whoami, tcode, NULL);
                    ERROR(tcode);
                }
 
@@ -307,7 +307,7 @@ EvalVolumeSet2(struct bc_config *aconfig,
                            if ((strcmp(&entries[e].name[l], ".backup") == 0)
                                || (strcmp(&entries[e].name[l], ".readonly")
                                    == 0)
-                               || (strcmp(&entries[e].name[l], "") == 0))
+                               || (strcmp(&entries[e].name[l], NULL) == 0))
                                add = 0;
                        }
                    }
@@ -318,14 +318,14 @@ EvalVolumeSet2(struct bc_config *aconfig,
                    tvd = (struct bc_volumeDump *)
                        malloc(sizeof(struct bc_volumeDump));
                    if (!tvd) {
-                       afs_com_err(whoami, BC_NOMEM, "");
+                       afs_com_err(whoami, BC_NOMEM, NULL);
                        ERROR(BC_NOMEM);
                    }
                    memset(tvd, 0, sizeof(*tvd));
 
                    tvd->name = (char *)malloc(strlen(entries[e].name) + 10);
                    if (!(tvd->name)) {
-                       afs_com_err(whoami, BC_NOMEM, "");
+                       afs_com_err(whoami, BC_NOMEM, NULL);
                        free(tvd);
                        ERROR(BC_NOMEM);
                    }
@@ -581,7 +581,7 @@ EvalVolumeSet1(struct bc_config *aconfig,
                                 entry.serverPartition[foundentry], &servers,
                                 &ss, &ps);
                if (code) {
-                   afs_com_err(whoami, code, "");
+                   afs_com_err(whoami, code, NULL);
                    return (code);
                }
 
@@ -589,14 +589,14 @@ EvalVolumeSet1(struct bc_config *aconfig,
                tvd = (struct bc_volumeDump *)
                    malloc(sizeof(struct bc_volumeDump));
                if (!tvd) {
-                   afs_com_err(whoami, BC_NOMEM, "");
+                   afs_com_err(whoami, BC_NOMEM, NULL);
                    return (BC_NOMEM);
                }
                memset(tvd, 0, sizeof(*tvd));
 
                tvd->name = (char *)malloc(strlen(entry.name) + 10);
                if (!(tvd->name)) {
-                   afs_com_err(whoami, BC_NOMEM, "");
+                   afs_com_err(whoami, BC_NOMEM, NULL);
                    free(tvd);
                    return (BC_NOMEM);
                }
@@ -744,7 +744,7 @@ bc_CopyString(char *astring)
     tlen = strlen(astring);
     tp = (char *)malloc(tlen + 1);     /* don't forget the terminating null */
     if (!tp) {
-       afs_com_err(whoami, BC_NOMEM, "");
+       afs_com_err(whoami, BC_NOMEM, NULL);
        return (tp);
     }
     strcpy(tp, astring);
@@ -777,7 +777,7 @@ concatParams(struct cmd_item *itemPtr)
 
     string = (char *)malloc(length);   /* allocate the string */
     if (!string) {
-       afs_com_err(whoami, BC_NOMEM, "");
+       afs_com_err(whoami, BC_NOMEM, NULL);
        return (NULL);
     }
     string[0] = 0;
@@ -1195,14 +1195,14 @@ bc_VolRestoreCmd(struct cmd_syndesc *as, void *arock)
        /* build list of volume items */
        tvol = (struct bc_volumeDump *)malloc(sizeof(struct bc_volumeDump));
        if (!tvol) {
-           afs_com_err(whoami, BC_NOMEM, "");
+           afs_com_err(whoami, BC_NOMEM, NULL);
            return BC_NOMEM;
        }
        memset(tvol, 0, sizeof(struct bc_volumeDump));
 
        tvol->name = (char *)malloc(VOLSER_MAXVOLNAME + 1);
        if (!tvol->name) {
-           afs_com_err(whoami, BC_NOMEM, "");
+           afs_com_err(whoami, BC_NOMEM, NULL);
            return BC_NOMEM;
        }
        strncpy(tvol->name, ti->data, VOLSER_OLDMAXVOLNAME);
@@ -1242,7 +1242,7 @@ bc_VolRestoreCmd(struct cmd_syndesc *as, void *arock)
            portCount++;
        ports = (afs_int32 *) malloc(portCount * sizeof(afs_int32));
        if (!ports) {
-           afs_com_err(whoami, BC_NOMEM, "");
+           afs_com_err(whoami, BC_NOMEM, NULL);
            return BC_NOMEM;
        }
 
@@ -1372,7 +1372,7 @@ bc_DiskRestoreCmd(struct cmd_syndesc *as, void *arock)
            portCount++;
        ports = (afs_int32 *) malloc(portCount * sizeof(afs_int32));
        if (!ports) {
-           afs_com_err(whoami, BC_NOMEM, "");
+           afs_com_err(whoami, BC_NOMEM, NULL);
            return BC_NOMEM;
        }
 
@@ -1543,7 +1543,7 @@ bc_VolsetRestoreCmd(struct cmd_syndesc *as, void *arock)
 
            tvol->name = (char *)malloc(VOLSER_MAXVOLNAME + 1);
            if (!tvol->name) {
-               afs_com_err(whoami, BC_NOMEM, "");
+               afs_com_err(whoami, BC_NOMEM, NULL);
                return BC_NOMEM;
            }
            strncpy(tvol->name, volume, VOLSER_OLDMAXVOLNAME);
@@ -1569,7 +1569,7 @@ bc_VolsetRestoreCmd(struct cmd_syndesc *as, void *arock)
            portCount++;
        ports = (afs_int32 *) malloc(portCount * sizeof(afs_int32));
        if (!ports) {
-           afs_com_err(whoami, BC_NOMEM, "");
+           afs_com_err(whoami, BC_NOMEM, NULL);
            return BC_NOMEM;
        }
 
@@ -1726,7 +1726,7 @@ bc_DumpCmd(struct cmd_syndesc *as, void *arock)
            portCount = 1;
            portp = (afs_int32 *) malloc(sizeof(afs_int32));
            if (!portp) {
-               afs_com_err(whoami, BC_NOMEM, "");
+               afs_com_err(whoami, BC_NOMEM, NULL);
                return BC_NOMEM;
            }
 
@@ -1807,7 +1807,7 @@ bc_DumpCmd(struct cmd_syndesc *as, void *arock)
            statusPtr->scheduledDump = atTime;
            statusPtr->cmdLine = (char *)malloc(length);
            if (!statusPtr->cmdLine) {
-               afs_com_err(whoami, BC_NOMEM, "");
+               afs_com_err(whoami, BC_NOMEM, NULL);
                return BC_NOMEM;
            }
 
@@ -1859,7 +1859,7 @@ bc_DumpCmd(struct cmd_syndesc *as, void *arock)
     if (loadfile) {
        loadFile = (char *)malloc(strlen(as->parms[6].items->data) + 1);
        if (!loadFile) {
-           afs_com_err(whoami, BC_NOMEM, "");
+           afs_com_err(whoami, BC_NOMEM, NULL);
            return BC_NOMEM;
        }
        strcpy(loadFile, as->parms[6].items->data);
@@ -2408,7 +2408,7 @@ deleteDump(afs_uint32 dumpid,             /* The dumpid to delete */
        /* Display the dumps that were deleted - includes appended dumps */
        for (i = 0; i < dumps.budb_dumpsList_len; i++)
            printf("     %u%s\n", dumps.budb_dumpsList_val[i],
-                  (i > 0) ? " Appended Dump" : "");
+                  (i > 0) ? " Appended Dump" : NULL);
        if (dumps.budb_dumpsList_val)
            free(dumps.budb_dumpsList_val);
     }
@@ -2838,7 +2838,7 @@ DBLookupByVolume(char *volumeName)
     }
 
     if (code)
-       afs_com_err(whoami, code, "");
+       afs_com_err(whoami, code, NULL);
     return (code);
 }
 
@@ -2926,7 +2926,7 @@ dumpInfo(afs_int32 dumpid, afs_int32 detailFlag)
     for (tapeNumber = dumpEntry.tapes.b; tapeNumber <= dumpEntry.tapes.maxTapes; tapeNumber++) {       /*f */
        tapeLinkPtr = (struct tapeLink *)malloc(sizeof(struct tapeLink));
        if (!tapeLinkPtr) {
-           afs_com_err(whoami, BC_NOMEM, "");
+           afs_com_err(whoami, BC_NOMEM, NULL);
            ERROR(BC_NOMEM);
        }
 
@@ -2978,7 +2978,7 @@ dumpInfo(afs_int32 dumpid, afs_int32 detailFlag)
                volumeLinkPtr =
                    (struct volumeLink *)malloc(sizeof(struct volumeLink));
                if (!volumeLinkPtr) {
-                   afs_com_err(whoami, BC_NOMEM, "");
+                   afs_com_err(whoami, BC_NOMEM, NULL);
                    ERROR(BC_NOMEM);
                }
                memset(volumeLinkPtr, 0, sizeof(*volumeLinkPtr));

diff --git a/src/bucoord/dump.c b/src/bucoord/dump.c
index 855a63e..7747866 100644 (file)
--- a/src/bucoord/dump.c
+++ b/src/bucoord/dump.c
@@ -90,7 +90,7 @@ bc_Dumper(int aindex)
     volDesc =
        (struct tc_dumpDesc *)malloc(count * sizeof(struct tc_dumpDesc));
     if (!volDesc) {
-       afs_com_err(whoami, BC_NOMEM, "");
+       afs_com_err(whoami, BC_NOMEM, NULL);
        ERROR(BC_NOMEM);
     }
 

diff --git a/src/bucoord/restore.c b/src/bucoord/restore.c
index 690ef33..210471e 100644 (file)
--- a/src/bucoord/restore.c
+++ b/src/bucoord/restore.c
@@ -201,7 +201,7 @@ bc_Restorer(afs_int32 aindex)
     volumeEntries = (struct budb_volumeEntry *)
        malloc(MAXTAPESATONCE * sizeof(struct budb_volumeEntry));
     if (!volumeEntries) {
-       afs_com_err(whoami, BC_NOMEM, "");
+       afs_com_err(whoami, BC_NOMEM, NULL);
        ERROR(BC_NOMEM);
     }
 
@@ -279,7 +279,7 @@ bc_Restorer(afs_int32 aindex)
        if (!di) {
            di = (struct dumpinfo *)malloc(sizeof(struct dumpinfo));
            if (!di) {
-               afs_com_err(whoami, BC_NOMEM, "");
+               afs_com_err(whoami, BC_NOMEM, NULL);
                ERROR(BC_NOMEM);
            }
            memset(di, 0, sizeof(struct dumpinfo));
@@ -301,7 +301,7 @@ bc_Restorer(afs_int32 aindex)
        /* Create one and thread into list */
        vi = (struct volinfo *)malloc(sizeof(struct volinfo));
        if (!vi) {
-           afs_com_err(whoami, BC_NOMEM, "");
+           afs_com_err(whoami, BC_NOMEM, NULL);
            ERROR(BC_NOMEM);
        }
        memset(vi, 0, sizeof(struct volinfo));
@@ -309,7 +309,7 @@ bc_Restorer(afs_int32 aindex)
        vi->volname = (char *)malloc(strlen(vname) + 1);
        if (!vi->volname) {
            free(vi);
-           afs_com_err(whoami, BC_NOMEM, "");
+           afs_com_err(whoami, BC_NOMEM, NULL);
            ERROR(BC_NOMEM);
        }
 
@@ -482,7 +482,7 @@ bc_Restorer(afs_int32 aindex)
                            tle = (struct bc_tapeList *)
                                malloc(sizeof(struct bc_tapeList));
                            if (!tle) {
-                               afs_com_err(whoami, BC_NOMEM, "");
+                               afs_com_err(whoami, BC_NOMEM, NULL);
                                return (BC_NOMEM);
                            }
                            memset(tle, 0, sizeof(struct bc_tapeList));
@@ -492,7 +492,7 @@ bc_Restorer(afs_int32 aindex)
                                               + 1);
                            if (!tle->tapeName) {
                                free(tle);
-                               afs_com_err(whoami, BC_NOMEM, "");
+                               afs_com_err(whoami, BC_NOMEM, NULL);
                                return (BC_NOMEM);
                            }
 
@@ -537,7 +537,7 @@ bc_Restorer(afs_int32 aindex)
                            ti = (struct bc_tapeItem *)
                                malloc(sizeof(struct bc_tapeItem));
                            if (!ti) {
-                               afs_com_err(whoami, BC_NOMEM, "");
+                               afs_com_err(whoami, BC_NOMEM, NULL);
                                return (BC_NOMEM);
                            }
                            memset(ti, 0, sizeof(struct bc_tapeItem));
@@ -547,7 +547,7 @@ bc_Restorer(afs_int32 aindex)
                                               + 1);
                            if (!ti->volumeName) {
                                free(ti);
-                               afs_com_err(whoami, BC_NOMEM, "");
+                               afs_com_err(whoami, BC_NOMEM, NULL);
                                return (BC_NOMEM);
                            }
 
@@ -637,7 +637,7 @@ bc_Restorer(afs_int32 aindex)
        (struct tc_restoreDesc *)malloc(nentries *
                                        sizeof(struct tc_restoreDesc));
     if (!tcarray) {
-       afs_com_err(whoami, BC_NOMEM, "");
+       afs_com_err(whoami, BC_NOMEM, NULL);
        ERROR(BC_NOMEM);
     }
     memset(tcarray, 0, nentries * sizeof(struct tc_restoreDesc));

diff --git a/src/bucoord/ubik_db_if.c b/src/bucoord/ubik_db_if.c
index f769d95..2588867 100644 (file)
--- a/src/bucoord/ubik_db_if.c
+++ b/src/bucoord/ubik_db_if.c
@@ -850,8 +850,8 @@ vldbClientInit(int noAuthFlag, int localauth, char *cellName,
            code =
                ktc_GetToken(&sname, ttoken, sizeof(struct ktc_token), NULL);
            if (code) {
-               afs_com_err(whoami, code, 0,
-                       "; Can't get AFS tokens - running unauthenticated");
+               afs_com_err(whoami, code,
+                           "; Can't get AFS tokens - running unauthenticated");
            } else {
                if ((ttoken->kvno < 0) || (ttoken->kvno > 256))
                    afs_com_err(whoami, 0,
@@ -1024,7 +1024,7 @@ udbClientInit(int noAuthFlag, int localauth, char *cellName)
     if (!udbHandle.uh_secobj) {
        afs_com_err(whoami, 0,
                "Can't create a security object with security index %d",
-               udbHandle.uh_secobj);
+               udbHandle.uh_scIndex);
        ERROR(-1);
     }
 

diff --git a/src/comerr/com_err.h b/src/comerr/com_err.h
index 15a8a3f..7e357ff 100644 (file)
--- a/src/comerr/com_err.h
+++ b/src/comerr/com_err.h
@@ -12,9 +12,13 @@
 
 #include <stdarg.h>
 
-extern void afs_com_err(const char *, afs_int32, const char *, ...);
+extern void afs_com_err(const char *, afs_int32, const char *, ...)
+    AFS_ATTRIBUTE_FORMAT(__printf__, 3, 4);
+
 extern void afs_com_err_va(const char *whoami, afs_int32 code, const char *fmt,
-                      va_list args);
+                      va_list args)
+    AFS_ATTRIBUTE_FORMAT(__printf__, 3, 0);
+
 extern const char *afs_error_table_name(afs_int32);
 extern const char *afs_error_message(afs_int32);
 extern

diff --git a/src/kauth/admin_tools.c b/src/kauth/admin_tools.c
index 4584264..adc7e3c 100644 (file)
--- a/src/kauth/admin_tools.c
+++ b/src/kauth/admin_tools.c
@@ -520,7 +520,7 @@ ka_islocked(char *name, char *instance, afs_uint32 * when)
                          0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
        if (code) {
            if (seriouserror(code))
-               afs_com_err(whoami, code, "");
+               afs_com_err(whoami, code, NULL);
        } else if (tempwhen) {  /* user is locked */
            if (!*when || tempwhen < *when) {
                *when = tempwhen;

diff --git a/src/ptserver/db_verify.c b/src/ptserver/db_verify.c
index 2ea33f9..533d153 100644 (file)
--- a/src/ptserver/db_verify.c
+++ b/src/ptserver/db_verify.c
@@ -1295,7 +1295,8 @@ CheckPrDatabase(struct misc_data *misc)   /* info & statistics */
     n = eof / sizeof(struct prentry);
     if ((eof < 0) || (n * sizeof(struct prentry) != eof)) {
        code = PRDBBAD;
-       afs_com_err(whoami, code, "eof ptr no good: eof=%d, sizeof(prentry)=%d",
+       afs_com_err(whoami, code,
+                   "eof ptr no good: eof=%d, sizeof(prentry)=%" AFS_SIZET_FMT,
                eof, sizeof(struct prentry));
       abort:
        return code;

diff --git a/src/uss/uss_kauth.c b/src/uss/uss_kauth.c
index 95c7c04..78bd8c8 100644 (file)
--- a/src/uss/uss_kauth.c
+++ b/src/uss/uss_kauth.c
@@ -706,8 +706,8 @@ uss_kauth_SetFields(char *username, char *expirestring, char *reuse,
                    "Must specify one of the optional parameters. Continuing...\n");
 
        if (code) {
-           afs_com_err(uss_whoami, code, "calling KAM_SetFields for %s.%s",
-                   username, instance);
+           afs_com_err(uss_whoami, code, "calling KAM_SetFields for %s",
+                   username);
 
            return (code);
        }