| Message ID | 20250106134606.115334-1-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | handshake: don't print NULL pmksa pointer | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| prestwoj/iwd-alpine-ci-fetch | success | Fetch PR |
| prestwoj/iwd-ci-gitlint | success | GitLint |
| prestwoj/iwd-ci-fetch | success | Fetch PR |
| prestwoj/iwd-alpine-ci-setupell | success | Prep - Setup ELL |
| prestwoj/iwd-ci-setupell | success | Prep - Setup ELL |
| prestwoj/iwd-ci-incremental_build | success | Incremental build not run PASS |
| prestwoj/iwd-alpine-ci-makedistcheck | success | Make Distcheck |
| prestwoj/iwd-alpine-ci-incremental_build | success | Incremental build not run PASS |
| prestwoj/iwd-ci-build | success | Build - Configure |
| prestwoj/iwd-alpine-ci-build | success | Build - Configure |
| prestwoj/iwd-ci-clang | success | clang PASS |
| prestwoj/iwd-alpine-ci-makecheckvalgrind | success | Make Check w/Valgrind |
| prestwoj/iwd-ci-makecheckvalgrind | success | Make Check w/Valgrind |
| prestwoj/iwd-alpine-ci-makecheck | success | Make Check |
| prestwoj/iwd-ci-makecheck | success | Make Check |
| prestwoj/iwd-ci-makedistcheck | success | Make Distcheck |
| prestwoj/iwd-ci-testrunner | success | test-runner PASS |
Hi James, On 1/6/25 7:46 AM, James Prestwood wrote: > This is undefined behavior so if no pmksa is found don't print. > --- > src/handshake.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/handshake.c b/src/handshake.c > index f73f91d1..bee76b26 100644 > --- a/src/handshake.c > +++ b/src/handshake.c > @@ -1272,11 +1272,11 @@ void handshake_state_cache_pmksa(struct handshake_state *s) > { > struct pmksa *pmksa = handshake_state_steal_pmksa(s); > > - l_debug("%p", pmksa); > - Why is this behavior undefined? Would it not just print 0x0000...? > if (!pmksa) > return; > > + l_debug("%p", pmksa); > + > if (L_WARN_ON(pmksa_cache_put(pmksa) < 0)) > l_free(pmksa); > } Regards, -Denis
Hi Denis, On 1/6/25 7:14 AM, Denis Kenzior wrote: > Hi James, > > On 1/6/25 7:46 AM, James Prestwood wrote: >> This is undefined behavior so if no pmksa is found don't print. >> --- >> src/handshake.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/src/handshake.c b/src/handshake.c >> index f73f91d1..bee76b26 100644 >> --- a/src/handshake.c >> +++ b/src/handshake.c >> @@ -1272,11 +1272,11 @@ void handshake_state_cache_pmksa(struct >> handshake_state *s) >> { >> struct pmksa *pmksa = handshake_state_steal_pmksa(s); >> - l_debug("%p", pmksa); >> - > > Why is this behavior undefined? Would it not just print 0x0000...? I had a brain fart, I was thinking for some reason it was using %s not %p. So yes, this is a valid input to the %p formatter. I can alter the commit description, but I do think it still makes sense to move the print, or at least modify it to make it clear no PMKSA was cached, rather than just printing "nul" or "nil" or "0x0000". > >> if (!pmksa) >> return; >> + l_debug("%p", pmksa); >> + >> if (L_WARN_ON(pmksa_cache_put(pmksa) < 0)) >> l_free(pmksa); >> } > > Regards, > -Denis
Hi James, >> >> Why is this behavior undefined? Would it not just print 0x0000...? > > I had a brain fart, I was thinking for some reason it was using %s not %p. So > yes, this is a valid input to the %p formatter. > > I can alter the commit description, but I do think it still makes sense to move > the print, or at least modify it to make it clear no PMKSA was cached, rather > than just printing "nul" or "nil" or "0x0000". Sure, that makes sense. Regards, -Denis
diff --git a/src/handshake.c b/src/handshake.c index f73f91d1..bee76b26 100644 --- a/src/handshake.c +++ b/src/handshake.c @@ -1272,11 +1272,11 @@ void handshake_state_cache_pmksa(struct handshake_state *s) { struct pmksa *pmksa = handshake_state_steal_pmksa(s); - l_debug("%p", pmksa); - if (!pmksa) return; + l_debug("%p", pmksa); + if (L_WARN_ON(pmksa_cache_put(pmksa) < 0)) l_free(pmksa); }