From: Simon Wilkinson Date: Sat, 7 Nov 2009 22:31:08 +0000 (+0000) Subject: Add printf format checks to afs_com_err() X-Git-Tag: openafs-devel-1_5_72~57 X-Git-Url: http://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=8229e668deee3eb00a295a8c9ea96a66b7049687;hp=68463b6ab9c664303692ac264871723c27bfc524 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 Tested-by: Derrick Brashear --- diff --git a/src/aklog/klog.c b/src/aklog/klog.c index 0647c68..8120c0f 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- a/src/comerr/com_err.h +++ b/src/comerr/com_err.h @@ -12,9 +12,13 @@ #include -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 --- 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 --- 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 --- 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); }