<html><head></head><body>Yeah those call sites! They'll busy loop, or cause their respective callers to, if you throw an ENOMEM error.<br><br>I call that a bug. You can't have errors in event handling - other than events masquerading as errors (EINTR...) and call site bugs (EBADF, EINVAL...).<br><br><div class="gmail_quote">Le 9 décembre 2020 11:28:19 GMT+02:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">On 2020-12-09 10:20, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">No it is not fine as already explained and as is obvious from reading <br>most call sites.<br></blockquote><br>What call sites are you talking about ? I'm talking about these:<br><br>* <br><a href="https://code.videolan.org/videolan/vlc/-/blob/master/modules/control/cli/cli.c#L515">https://code.videolan.org/videolan/vlc/-/blob/master/modules/control/cli/cli.c#L515</a><br> int fd = net_Accept(intf, sys->pi_socket_listen);<br> if (fd == -1)<br> continue;<br><br>* <br><a href="https://code.videolan.org/videolan/vlc/-/blob/master/modules/control/cli/cli.c#L756">https://code.videolan.org/videolan/vlc/-/blob/master/modules/control/cli/cli.c#L756</a><br> p_sys->i_socket =<br> net_Accept( p_intf, p_sys->pi_socket_listen );<br> if( p_sys->i_socket == -1 ) continue;<br><br>* <br><a href="https://code.videolan.org/videolan/vlc/-/blob/master/modules/stream_out/rtp.c#L1414">https://code.videolan.org/videolan/vlc/-/blob/master/modules/stream_out/rtp.c#L1414</a><br> int fd = net_Accept( id->p_stream, id->listen.fd );<br> if( fd == -1 )<br> continue;<br><br>* <br><a href="https://code.videolan.org/videolan/vlc/-/blob/master/modules/lua/libs/net.c#L240">https://code.videolan.org/videolan/vlc/-/blob/master/modules/lua/libs/net.c#L240</a><br> int i_fd = net_Accept( p_this, *ppi_fd );<br><br> lua_pushinteger( L, vlclua_fd_map_safe( L, i_fd ) );<br><br>There is no other net_Accept() calls.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">-1<br><br>Le 9 décembre 2020 11:11:20 GMT+02:00, Steve Lhomme <robux4@ycbcr.xyz> a <br>écrit :<br><br> On 2020-12-09 10:06, Rémi Denis-Courmont wrote:<br><br> Callers don't expect and can't handle ENOMEM error here. If you<br> don't<br> understand, abstain from commenting instead of making stupid<br> wrong comments.<br><br><br> "Callers don't expect errors here" is very different than "Callers don't<br> expect and can't handle ENOMEM error here".<br><br> And the net_Accept() documentation doesn't mention that. And in fact,<br> apart from Lua that just passes on the value, all the callers check for<br> -1 and don't care about the actual type of errors.<br><br> So this fine is perfectly fine.<br><br> Le 9 décembre 2020 08:50:43 GMT+02:00, Steve Lhomme<br> <robux4@ycbcr.xyz> a<br> écrit :<br><br> This is a review of your comment which is a wrong assertion.<br> Keep on topic.<br><br> On 2020-12-08 16:43, Rémi Denis-Courmont wrote:<br><br> Please keep your pointless sarcasms out of the code review.<br><br> Le 8 décembre 2020 17:20:37 GMT+02:00, Steve Lhomme<br> <robux4@ycbcr.xyz> a<br> écrit :<br><br> On 2020-12-08 16:12, Rémi Denis-Courmont wrote:<br><br> Callers don't expect errors here. Polling just has to work.<br><br> -1<br><br><br> yes, there are plenty of return -1, as seen in the patch.<br><br> Le 8 décembre 2020 16:19:13 GMT+02:00, "Hugo Beauzée-Luyssen"<br> <hugo@beauzee.fr> a écrit :<hr> src/network/io.c | 7 ++++++-<br> 1 file changed, 6 insertions(+), 1 deletion(-)<br><br> diff --git a/src/network/io.c b/src/network/io.c<br> index 5285edc169..d4f15fe115 100644<br> --- a/src/network/io.c<br> +++ b/src/network/io.c<br> @@ -313,7 +313,9 @@ int net_Accept(vlc_object_t *obj, int *fds)<br> while (fds[n] != -1)<br> n++;<br><br> - struct pollfd ufd[n];<br> + struct pollfd *ufd = malloc(n * sizeof((*ufd)));<br> + if (!ufd)<br> + return -1;<br> /* Initialize file descriptor set */<br> for (unsigned i = 0; i < n; i++)<br> {<br> @@ -328,6 +330,7 @@ int net_Accept(vlc_object_t *obj, int *fds)<br> if (net_errno != EINTR)<br> {<br> msg_Err(obj, "poll error: %s", vlc_strerror_c(net_errno));<br> + free(ufd);<br> return -1;<br> }<br> }<br> @@ -359,9 +362,11 @@ int net_Accept(vlc_object_t *obj, int *fds)<br> */<br> memmove(fds + i, fds + i + 1, n - (i + 1));<br> fds[n - 1] = sfd;<br> + free(ufd);<br> return fd;<br> }<br> }<br> + free(ufd);<br> return -1;<br> }<br><br><br><br> -- <br> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez<br> excuser<br> ma brièveté.<hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>>><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>>>><hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>>><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>>>><br><br><br> -- <br> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez<br> excuser<br> ma brièveté.<hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>>><hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>>><br><br><br> -- <br> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez<br> excuser<br> ma brièveté.<hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>><hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a> <<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>><br><br><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser <br>ma brièveté.<hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>