Hello Jan,

Jan Klemkow <j.klem...@wemelug.de> 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 via email to