diff: simplify MGETHDR error handling in tcp_output

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

diff: simplify MGETHDR error handling in tcp_output

Jan Klemkow
Hi,

the following diff simplifies the error handling of MGETHDR() in
tcp_output().  Jumps earlier to out, prevents a double check of NULL and
makes the code more readable.

OK?

Bye,
Jan

Index: netinet/tcp_output.c
===================================================================
RCS file: /cvs/src/sys/netinet/tcp_output.c,v
retrieving revision 1.128
diff -u -p -r1.128 tcp_output.c
--- netinet/tcp_output.c 10 Nov 2018 18:40:34 -0000 1.128
+++ netinet/tcp_output.c 6 Nov 2019 14:34:40 -0000
@@ -652,17 +652,17 @@ send:
  m->m_data -= hdrlen;
 #else
  MGETHDR(m, M_DONTWAIT, MT_HEADER);
- if (m != NULL && max_linkhdr + hdrlen > MHLEN) {
+ if (m == NULL) {
+ error = ENOBUFS;
+ goto out;
+ }
+ if (max_linkhdr + hdrlen > MHLEN) {
  MCLGET(m, M_DONTWAIT);
  if ((m->m_flags & M_EXT) == 0) {
  m_freem(m);
  m = NULL;
  }
  }
- if (m == NULL) {
- error = ENOBUFS;
- goto out;
- }
  m->m_data += max_linkhdr;
  m->m_len = hdrlen;
  if (len <= m_trailingspace(m)) {
@@ -701,16 +701,16 @@ send:
  tcpstat_inc(tcps_sndwinup);
 
  MGETHDR(m, M_DONTWAIT, MT_HEADER);
- if (m != NULL && max_linkhdr + hdrlen > MHLEN) {
+ if (m == NULL) {
+ error = ENOBUFS;
+ goto out;
+ }
+ if (max_linkhdr + hdrlen > MHLEN) {
  MCLGET(m, M_DONTWAIT);
  if ((m->m_flags & M_EXT) == 0) {
  m_freem(m);
  m = NULL;
  }
- }
- if (m == NULL) {
- error = ENOBUFS;
- goto out;
  }
  m->m_data += max_linkhdr;
  m->m_len = hdrlen;

Reply | Threaded
Open this post in threaded view
|

Re: diff: simplify MGETHDR error handling in tcp_output

Lucas-2
Hello Jan,

Jan Klemkow <[hidden email]> wrote:

> Hi,
>
> the following diff simplifies the error handling of MGETHDR() in
> tcp_output().  Jumps earlier to out, prevents a double check of NULL and
> makes the code more readable.
>
> OK?
>
> Bye,
> Jan
>
> Index: netinet/tcp_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/tcp_output.c,v
> retrieving revision 1.128
> diff -u -p -r1.128 tcp_output.c
> --- netinet/tcp_output.c 10 Nov 2018 18:40:34 -0000 1.128
> +++ netinet/tcp_output.c 6 Nov 2019 14:34:40 -0000
> @@ -652,17 +652,17 @@ send:
>   m->m_data -= hdrlen;
>  #else
>   MGETHDR(m, M_DONTWAIT, MT_HEADER);
> - if (m != NULL && max_linkhdr + hdrlen > MHLEN) {
> + if (m == NULL) {
> + error = ENOBUFS;
> + goto out;
> + }
> + if (max_linkhdr + hdrlen > MHLEN) {
>   MCLGET(m, M_DONTWAIT);
>   if ((m->m_flags & M_EXT) == 0) {
>   m_freem(m);
>   m = NULL;
>   }
>   }
> - if (m == NULL) {
> - error = ENOBUFS;
> - goto out;
> - }
>   m->m_data += max_linkhdr;
>   m->m_len = hdrlen;

I might be missing something, but m can be NULL here, if (m->m_flags &
M_EXT) == 0.

>   if (len <= m_trailingspace(m)) {
> @@ -701,16 +701,16 @@ send:
>   tcpstat_inc(tcps_sndwinup);
>  
>   MGETHDR(m, M_DONTWAIT, MT_HEADER);
> - if (m != NULL && max_linkhdr + hdrlen > MHLEN) {
> + if (m == NULL) {
> + error = ENOBUFS;
> + goto out;
> + }
> + if (max_linkhdr + hdrlen > MHLEN) {
>   MCLGET(m, M_DONTWAIT);
>   if ((m->m_flags & M_EXT) == 0) {
>   m_freem(m);
>   m = NULL;
>   }
> - }
> - if (m == NULL) {
> - error = ENOBUFS;
> - goto out;
>   }
>   m->m_data += max_linkhdr;
>   m->m_len = hdrlen;

And same here.

-Lucas

Reply | Threaded
Open this post in threaded view
|

Re: diff: simplify MGETHDR error handling in tcp_output

Jan Klemkow
Hi Lucas,

On Wed, Nov 06, 2019 at 08:28:43PM +0000, Lucas wrote:

> Jan Klemkow <[hidden email]> wrote:
> > the following diff simplifies the error handling of MGETHDR() in
> > tcp_output().  Jumps earlier to out, prevents a double check of NULL and
> > makes the code more readable.
> >
> > OK?
> >
> > Bye,
> > Jan
> >
> > Index: netinet/tcp_output.c
> > ===================================================================
> > RCS file: /cvs/src/sys/netinet/tcp_output.c,v
> > retrieving revision 1.128
> > diff -u -p -r1.128 tcp_output.c
> > --- netinet/tcp_output.c 10 Nov 2018 18:40:34 -0000 1.128
> > +++ netinet/tcp_output.c 6 Nov 2019 14:34:40 -0000
> > @@ -652,17 +652,17 @@ send:
> >   m->m_data -= hdrlen;
> >  #else
> >   MGETHDR(m, M_DONTWAIT, MT_HEADER);
> > - if (m != NULL && max_linkhdr + hdrlen > MHLEN) {
> > + if (m == NULL) {
> > + error = ENOBUFS;
> > + goto out;
> > + }
> > + if (max_linkhdr + hdrlen > MHLEN) {
> >   MCLGET(m, M_DONTWAIT);
> >   if ((m->m_flags & M_EXT) == 0) {
> >   m_freem(m);
> >   m = NULL;
> >   }
> >   }
> > - if (m == NULL) {
> > - error = ENOBUFS;
> > - goto out;
> > - }
> >   m->m_data += max_linkhdr;
> >   m->m_len = hdrlen;
>
> I might be missing something, but m can be NULL here, if (m->m_flags &
> M_EXT) == 0.

Yes, you are right.  I missed that.

Thank,
Jan