On Tue, 6 Feb 2018, Alexander Bluhm wrote:
> On Tue, Feb 06, 2018 at 11:04:51AM +1300, Richard Procter wrote:
> > > @@ -657,12 +667,13 @@ ah_input(struct mbuf *m, struct tdb *tdb
> > > m_copyback(m, skip + rplen, ahx->authsize, ipseczeroes, M_NOWAIT);
> > >
> > > /* "Massage" the packet headers for crypto processing. */
> > > - if ((btsx = ah_massage_headers(&m, tdb->tdb_dst.sa.sa_family,
> > > - skip, ahx->type, 0)) != 0) {
> > > + error = ah_massage_headers(&m, tdb->tdb_dst.sa.sa_family, skip,
> > > + ahx->type, 0);
> > > + if (error) {
> > > /* mbuf will be free'd by callee. */
> >
> > This pre-existing comment muddled me. ah_massage_headers() has already
> > freed it on error.
>
> Although the code is correct, I also had to read the comment twice
> when I first saw it. I think the tense is wrong. The callee is
> ah_massage_headers, and there free has been called.
>
> Does this clarify it?
It does, but the grammar is a little odd. (English tenses are obscure when
I stop to think about them -- for instance, "he will be speckled" and "he
has been bespeckled" works but "he has been be-freed" doesn't for some
reason... I almost want to say "befree'n" but that sounds archaic.)
Anyways, I'll commit the simplification below. Having raised the issue, I
should have provided it in the first place.
Index: netinet/ip_ah.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_ah.c,v
retrieving revision 1.135
diff -u -p -u -r1.135 ip_ah.c
--- netinet/ip_ah.c 6 Feb 2018 14:54:22 -0000 1.135
+++ netinet/ip_ah.c 6 Feb 2018 20:01:57 -0000
@@ -670,7 +670,7 @@ ah_input(struct mbuf *m, struct tdb *tdb
error = ah_massage_headers(&m, tdb->tdb_dst.sa.sa_family, skip,
ahx->type, 0);
if (error) {
- /* mbuf will be free'd by callee. */
+ /* mbuf was freed by callee. */
free(tc, M_XDATA, 0);
crypto_freereq(crp);
return error;
@@ -1158,7 +1158,7 @@ ah_output(struct mbuf *m, struct tdb *td
error = ah_massage_headers(&m, tdb->tdb_dst.sa.sa_family, skip,
ahx->type, 1);
if (error) {
- /* mbuf will be free'd by callee. */
+ /* mbuf was freed by callee. */
free(tc, M_XDATA, 0);
crypto_freereq(crp);
return error;
> bluhm
>
> Index: netinet/ip_ah.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ah.c,v
> retrieving revision 1.135
> diff -u -p -r1.135 ip_ah.c
> --- netinet/ip_ah.c 6 Feb 2018 14:54:22 -0000 1.135
> +++ netinet/ip_ah.c 6 Feb 2018 15:14:29 -0000
> @@ -670,7 +670,7 @@ ah_input(struct mbuf *m, struct tdb *tdb
> error = ah_massage_headers(&m, tdb->tdb_dst.sa.sa_family, skip,
> ahx->type, 0);
> if (error) {
> - /* mbuf will be free'd by callee. */
> + /* mbuf has been be free'd by callee. */
> free(tc, M_XDATA, 0);
> crypto_freereq(crp);
> return error;
> @@ -1158,7 +1158,7 @@ ah_output(struct mbuf *m, struct tdb *td
> error = ah_massage_headers(&m, tdb->tdb_dst.sa.sa_family, skip,
> ahx->type, 1);
> if (error) {
> - /* mbuf will be free'd by callee. */
> + /* mbuf has been be free'd by callee. */
> free(tc, M_XDATA, 0);
> crypto_freereq(crp);
> return error;
>
