On Wed, 5 Feb 2020, Michael Forney wrote:> Hi Damien, > > On 11/27/19, Michael Forney <mforney at mforney.org> wrote: > > This can be caught with -Wpedantic, which seems to catch a few more > > instances of this, as well as passing non-void pointers to printf for > > the `%p` format specifier. Patches attached for these. > > Any issues with these three patches?Sorry for letting these slip. I've imported the MAKE_CLONE one. I'll look at fixing the %p warning after release, probably by removing all the %p formats :) The RB_GENERATE_STATIC one I'll also look at after release, by syncing openbsd-compat/sys-tree.h with upstream as that has already fixed the same problem. -d
On 2020-02-05, Damien Miller <djm at mindrot.org> wrote:> Sorry for letting these slip.No worries, thanks for looking :)> I've imported the MAKE_CLONE one. I'll look at fixing the %p warning > after release, probably by removing all the %p formats :) > > The RB_GENERATE_STATIC one I'll also look at after release, by syncing > openbsd-compat/sys-tree.h with upstream as that has already fixed the > same problem.Sounds good, thanks.
Michael Forney
2021-May-14 01:14 UTC
[PATCH v2 1/2] printf %p specifier requires `void *` argument
It is undefined behavior to call printf with an argument for %p that does not have type `void *` (see C99 7.19.6.1p9). --- On 2020-02-05, Damien Miller <djm at mindrot.org> wrote:> I'll look at fixing the %p warning > after release, probably by removing all the %p formats :) > > The RB_GENERATE_STATIC one I'll also look at after release, by syncing > openbsd-compat/sys-tree.h with upstream as that has already fixed the > same problem.Resending these patches rebased on latest master. monitor.c | 4 ++-- session.c | 2 +- ssh-pkcs11-helper.c | 2 +- ssh-pkcs11.c | 16 +++++++++------- sshbuf-misc.c | 2 +- 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/monitor.c b/monitor.c index 9d23d823..d0eecdf1 100644 --- a/monitor.c +++ b/monitor.c @@ -1163,7 +1163,7 @@ mm_answer_keyallowed(struct ssh *ssh, int sock, struct sshbuf *m) (r = sshbuf_get_u32(m, &pubkey_auth_attempt)) != 0) fatal_fr(r, "parse"); - debug3_f("key_from_blob: %p", key); + debug3_f("key_from_blob: %p", (void *)key); if (key != NULL && authctxt->valid) { /* These should not make it past the privsep child */ @@ -1434,7 +1434,7 @@ mm_answer_keyverify(struct ssh *ssh, int sock, struct sshbuf *m) ret = sshkey_verify(key, signature, signaturelen, data, datalen, sigalg, ssh->compat, &sig_details); - debug3_f("%s %p signature %s%s%s", auth_method, key, + debug3_f("%s %p signature %s%s%s", auth_method, (void *)key, (ret == 0) ? "verified" : "unverified", (ret != 0) ? ": " : "", (ret != 0) ? ssh_err(ret) : ""); diff --git a/session.c b/session.c index c02f7d25..bab1a6d1 100644 --- a/session.c +++ b/session.c @@ -1788,7 +1788,7 @@ session_dump(void) s->used, s->next_unused, s->self, - s, + (void *)s, s->chanid, (long)s->pid); } diff --git a/ssh-pkcs11-helper.c b/ssh-pkcs11-helper.c index a9a6fe38..8f3060d9 100644 --- a/ssh-pkcs11-helper.c +++ b/ssh-pkcs11-helper.c @@ -98,7 +98,7 @@ lookup_key(struct sshkey *k) struct pkcs11_keyinfo *ki; TAILQ_FOREACH(ki, &pkcs11_keylist, next) { - debug("check %p %s %s", ki, ki->providername, ki->label); + debug("check %p %s %s", (void *)ki, ki->providername, ki->label); if (sshkey_equal(k, ki->key)) return (ki->key); } diff --git a/ssh-pkcs11.c b/ssh-pkcs11.c index 844aa9ff..72ff6bef 100644 --- a/ssh-pkcs11.c +++ b/ssh-pkcs11.c @@ -112,7 +112,7 @@ pkcs11_provider_finalize(struct pkcs11_provider *p) CK_ULONG i; debug("pkcs11_provider_finalize: %p refcount %d valid %d", - p, p->refcount, p->valid); + (void *)p, p->refcount, p->valid); if (!p->valid) return; for (i = 0; i < p->nslots; i++) { @@ -135,10 +135,12 @@ pkcs11_provider_finalize(struct pkcs11_provider *p) static void pkcs11_provider_unref(struct pkcs11_provider *p) { - debug("pkcs11_provider_unref: %p refcount %d", p, p->refcount); + debug("pkcs11_provider_unref: %p refcount %d", (void *)p, p->refcount); if (--p->refcount <= 0) { - if (p->valid) - error("pkcs11_provider_unref: %p still valid", p); + if (p->valid) { + error("pkcs11_provider_unref: %p still valid", + (void *)p); + } free(p->name); free(p->slotlist); free(p->slotinfo); @@ -166,7 +168,7 @@ pkcs11_provider_lookup(char *provider_id) struct pkcs11_provider *p; TAILQ_FOREACH(p, &pkcs11_providers, next) { - debug("check %p %s", p, p->name); + debug("check %p %s", (void *)p, p->name); if (!strcmp(provider_id, p->name)) return (p); } @@ -338,7 +340,7 @@ pkcs11_check_obj_bool_attrib(struct pkcs11_key *k11, CK_OBJECT_HANDLE obj, } *val = flag != 0; debug_f("provider %p slot %lu object %lu: attrib %lu = %d", - k11->provider, k11->slotidx, obj, type, *val); + (void *)k11->provider, k11->slotidx, obj, type, *val); return (0); } @@ -430,7 +432,7 @@ pkcs11_rsa_private_encrypt(int flen, const u_char *from, u_char *to, RSA *rsa, int rval = -1; if ((k11 = RSA_get_ex_data(rsa, rsa_idx)) == NULL) { - error("RSA_get_ex_data failed for rsa %p", rsa); + error("RSA_get_ex_data failed for rsa %p", (void *)rsa); return (-1); } diff --git a/sshbuf-misc.c b/sshbuf-misc.c index afaab8d6..1a9f6022 100644 --- a/sshbuf-misc.c +++ b/sshbuf-misc.c @@ -65,7 +65,7 @@ sshbuf_dump_data(const void *s, size_t len, FILE *f) void sshbuf_dump(const struct sshbuf *buf, FILE *f) { - fprintf(f, "buffer %p len = %zu\n", buf, sshbuf_len(buf)); + fprintf(f, "buffer %p len = %zu\n", (void *)buf, sshbuf_len(buf)); sshbuf_dump_data(sshbuf_ptr(buf), sshbuf_len(buf), f); } -- 2.31.1
Michael Forney
2021-May-14 01:14 UTC
[PATCH v2 2/2] Remove trailing semicolon after RB_GENERATE_STATIC
This expands to a series of function definitions, so the semicolon is
not necessary (in fact, it is not allowed in ISO C).
---
krl.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/krl.c b/krl.c
index 5612e774..c75279f9 100644
--- a/krl.c
+++ b/krl.c
@@ -61,7 +61,7 @@ struct revoked_serial {
};
static int serial_cmp(struct revoked_serial *a, struct revoked_serial *b);
RB_HEAD(revoked_serial_tree, revoked_serial);
-RB_GENERATE_STATIC(revoked_serial_tree, revoked_serial, tree_entry,
serial_cmp);
+RB_GENERATE_STATIC(revoked_serial_tree, revoked_serial, tree_entry, serial_cmp)
/* Tree of key IDs */
struct revoked_key_id {
@@ -70,7 +70,7 @@ struct revoked_key_id {
};
static int key_id_cmp(struct revoked_key_id *a, struct revoked_key_id *b);
RB_HEAD(revoked_key_id_tree, revoked_key_id);
-RB_GENERATE_STATIC(revoked_key_id_tree, revoked_key_id, tree_entry,
key_id_cmp);
+RB_GENERATE_STATIC(revoked_key_id_tree, revoked_key_id, tree_entry, key_id_cmp)
/* Tree of blobs (used for keys and fingerprints) */
struct revoked_blob {
@@ -80,7 +80,7 @@ struct revoked_blob {
};
static int blob_cmp(struct revoked_blob *a, struct revoked_blob *b);
RB_HEAD(revoked_blob_tree, revoked_blob);
-RB_GENERATE_STATIC(revoked_blob_tree, revoked_blob, tree_entry, blob_cmp);
+RB_GENERATE_STATIC(revoked_blob_tree, revoked_blob, tree_entry, blob_cmp)
/* Tracks revoked certs for a single CA */
struct revoked_certs {
--
2.31.1