Message ID | 20240708210912.566-2-chipitsine@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | handle strdup errors | expand |
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, > + } > } > } > }
пн, 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
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,
пн, 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
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,
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 --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"); + } } } }
Signed-off-by: Ilia Shipitsin <chipitsine@gmail.com> --- src/openvpn/init.c | 4 ++++ 1 file changed, 4 insertions(+)