diff mbox series

[Openvpn-devel,1/5] src/openvpn/init.c: handle strdup failures

Message ID 20240708210912.566-2-chipitsine@gmail.com
State Accepted
Headers
Series handle strdup errors |

Commit Message

Илья Шипицин July 8, 2024, 9:08 p.m. UTC 
  Signed-off-by: Ilia Shipitsin <chipitsine@gmail.com>
---
 src/openvpn/init.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Antonio Quartulli July 8, 2024, 9:14 p.m. UTC | #1 
Hi,

On 08/07/2024 23:08, Ilia Shipitsin wrote:
> Signed-off-by: Ilia Shipitsin <chipitsine@gmail.com>
> ---
>   src/openvpn/init.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index a49e5639..59205ba6 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -4967,6 +4967,10 @@ write_pid_file(const char *filename, const char *chroot_dir)
>           if (!chroot_dir)
>           {
>               saved_pid_file_name = strdup(filename);
> +            if (!saved_pid_file_name)
> +            {
> +                msg( M_FATAL, "Failed allocate memory saved_pid_file_name");

patchset looks great, but (!!) there should be no space after the 
opening parenthesis..

Cheers,

> +            }
>           }
>       }
>   }
Илья Шипицин July 8, 2024, 9:44 p.m. UTC | #2 
пн, 8 июл. 2024 г. в 23:13, Antonio Quartulli <a@unstable.cc>:
>
> Hi,
>
> On 08/07/2024 23:08, Ilia Shipitsin wrote:
> > Signed-off-by: Ilia Shipitsin <chipitsine@gmail.com>
> > ---
> >   src/openvpn/init.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> > index a49e5639..59205ba6 100644
> > --- a/src/openvpn/init.c
> > +++ b/src/openvpn/init.c
> > @@ -4967,6 +4967,10 @@ write_pid_file(const char *filename, const char *chroot_dir)
> >           if (!chroot_dir)
> >           {
> >               saved_pid_file_name = strdup(filename);
> > +            if (!saved_pid_file_name)
> > +            {
> > +                msg( M_FATAL, "Failed allocate memory saved_pid_file_name");
>
> patchset looks great, but (!!) there should be no space after the
> opening parenthesis..

I just followed code style already present in repo

$ grep -r 'msg( M_FATAL' .
grep: ./.git/objects/pack/pack-d09f3eabac42224b1dc52ffb4f53d9af23bc5b7d.pack:
binary file matches
./src/openvpn/auth_token.c:        msg( M_FATAL, "Failed to get enough
randomness for "
./src/openvpn/tun.c:            msg( M_FATAL, "init_tun: problem
converting IPv6 ifconfig addresses %s and %s to binary",
ifconfig_ipv6_local_parm, ifconfig_ipv6_remote_parm );
./src/openvpn/tun.c:            msg( M_FATAL, "cannot find unused tap device" );
./src/openvpn/tun.c:            msg( M_FATAL, "TAP device name must be
'--dev tapNNNN'" );

also, uncrustify GHA jobs agreed that it is no formatting violation


>
> Cheers,
>
> > +            }
> >           }
> >       }
> >   }
>
> --
> Antonio Quartulli
Antonio Quartulli July 8, 2024, 9:49 p.m. UTC | #3 
Hi,

On 08/07/2024 23:44, Илья Шипицин wrote:
>>> +                msg( M_FATAL, "Failed allocate memory saved_pid_file_name");
>>
>> patchset looks great, but (!!) there should be no space after the
>> opening parenthesis..
> 
> I just followed code style already present in repo
> 
> $ grep -r 'msg( M_FATAL' .
> grep: ./.git/objects/pack/pack-d09f3eabac42224b1dc52ffb4f53d9af23bc5b7d.pack:
> binary file matches
> ./src/openvpn/auth_token.c:        msg( M_FATAL, "Failed to get enough
> randomness for "
> ./src/openvpn/tun.c:            msg( M_FATAL, "init_tun: problem
> converting IPv6 ifconfig addresses %s and %s to binary",
> ifconfig_ipv6_local_parm, ifconfig_ipv6_remote_parm );
> ./src/openvpn/tun.c:            msg( M_FATAL, "cannot find unused tap device" );
> ./src/openvpn/tun.c:            msg( M_FATAL, "TAP device name must be
> '--dev tapNNNN'" );

Unfortunately those are unlucky leftovers that haven't been fixed yet:

$ grep -r 'msg(M_FATAL' . |wc -l
286

$ grep -r 'msg( M_FATAL' . |wc -l
4

> 
> also, uncrustify GHA jobs agreed that it is no formatting violation

doubly unfortunate as I think uncrustify can't force this or it gets 
confused somehow :(


Cheers,
Илья Шипицин July 8, 2024, 9:53 p.m. UTC | #4 
пн, 8 июл. 2024 г. в 23:47, Antonio Quartulli <a@unstable.cc>:
>
> Hi,
>
> On 08/07/2024 23:44, Илья Шипицин wrote:
> >>> +                msg( M_FATAL, "Failed allocate memory saved_pid_file_name");
> >>
> >> patchset looks great, but (!!) there should be no space after the
> >> opening parenthesis..
> >
> > I just followed code style already present in repo
> >
> > $ grep -r 'msg( M_FATAL' .
> > grep: ./.git/objects/pack/pack-d09f3eabac42224b1dc52ffb4f53d9af23bc5b7d.pack:
> > binary file matches
> > ./src/openvpn/auth_token.c:        msg( M_FATAL, "Failed to get enough
> > randomness for "
> > ./src/openvpn/tun.c:            msg( M_FATAL, "init_tun: problem
> > converting IPv6 ifconfig addresses %s and %s to binary",
> > ifconfig_ipv6_local_parm, ifconfig_ipv6_remote_parm );
> > ./src/openvpn/tun.c:            msg( M_FATAL, "cannot find unused tap device" );
> > ./src/openvpn/tun.c:            msg( M_FATAL, "TAP device name must be
> > '--dev tapNNNN'" );
>
> Unfortunately those are unlucky leftovers that haven't been fixed yet:
>
> $ grep -r 'msg(M_FATAL' . |wc -l
> 286
>
> $ grep -r 'msg( M_FATAL' . |wc -l
> 4
>
> >
> > also, uncrustify GHA jobs agreed that it is no formatting violation
>
> doubly unfortunate as I think uncrustify can't force this or it gets
> confused somehow :(

if those spaces makes you unhappy, I'm fine if you fix them during commit apply


>
>
> Cheers,
>
>
>
> --
> Antonio Quartulli
Antonio Quartulli July 8, 2024, 9:57 p.m. UTC | #5 
On 08/07/2024 23:53, Илья Шипицин wrote:
> пн, 8 июл. 2024 г. в 23:47, Antonio Quartulli <a@unstable.cc>:
>>
>> Hi,
>>
>> On 08/07/2024 23:44, Илья Шипицин wrote:
>>>>> +                msg( M_FATAL, "Failed allocate memory saved_pid_file_name");
>>>>
>>>> patchset looks great, but (!!) there should be no space after the
>>>> opening parenthesis..
>>>
>>> I just followed code style already present in repo
>>>
>>> $ grep -r 'msg( M_FATAL' .
>>> grep: ./.git/objects/pack/pack-d09f3eabac42224b1dc52ffb4f53d9af23bc5b7d.pack:
>>> binary file matches
>>> ./src/openvpn/auth_token.c:        msg( M_FATAL, "Failed to get enough
>>> randomness for "
>>> ./src/openvpn/tun.c:            msg( M_FATAL, "init_tun: problem
>>> converting IPv6 ifconfig addresses %s and %s to binary",
>>> ifconfig_ipv6_local_parm, ifconfig_ipv6_remote_parm );
>>> ./src/openvpn/tun.c:            msg( M_FATAL, "cannot find unused tap device" );
>>> ./src/openvpn/tun.c:            msg( M_FATAL, "TAP device name must be
>>> '--dev tapNNNN'" );
>>
>> Unfortunately those are unlucky leftovers that haven't been fixed yet:
>>
>> $ grep -r 'msg(M_FATAL' . |wc -l
>> 286
>>
>> $ grep -r 'msg( M_FATAL' . |wc -l
>> 4
>>
>>>
>>> also, uncrustify GHA jobs agreed that it is no formatting violation
>>
>> doubly unfortunate as I think uncrustify can't force this or it gets
>> confused somehow :(
> 
> if those spaces makes you unhappy, I'm fine if you fix them during commit apply

I don't think Gert like to change code lines while committing a patch 
(We can wait for him to say something)

However, I can also offer myself to send a patch with a full cleanup 
afterwards.

Cheers,
Gert Doering Sept. 9, 2024, 7:20 a.m. UTC | #6 
Acked-by: Gert Doering <gert@greenie.muc.de>

Taken the "patchset looks great" from Antonio as ACK, fixed the 
"msg( M_FATAL," space on the go (trivial whitespace fixes are
acceptable).

Not tested beyond minimal compile test and stare-at-code.

Your patch has been applied to the master branch.

commit b7c6920eab5d56067a69805f64239b8dd5a0ae27
Author: Ilia Shipitsin
Date:   Mon Jul 8 23:08:18 2024 +0200

     src/openvpn/init.c: handle strdup failures

     Signed-off-by: Ilia Shipitsin <chipitsine@gmail.com>
     Acked-by: Antonio Quartulli <a@unstable.cc>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20240708210912.566-2-chipitsine@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28884.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index a49e5639..59205ba6 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -4967,6 +4967,10 @@  write_pid_file(const char *filename, const char *chroot_dir)
         if (!chroot_dir)
         {
             saved_pid_file_name = strdup(filename);
+            if (!saved_pid_file_name)
+            {
+                msg( M_FATAL, "Failed allocate memory saved_pid_file_name");
+            }
         }
     }
 }