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