| Message ID | 20161124145550.GA87084@ns.kevlo.org |
|---|---|
| State | Changes Requested |
| Headers | show |
Le quartidi 4 frimaire, an CCXXV, Kevin Lo a écrit : > FreeBSD's socket calls require the sockaddr struct length to agree > with the address family, Linux does not. This patch fixes a failing > getnameinfo() call on FreeBSD. > > Signed-off-by: Kevin Lo <kevlo@kevlo.org> I looked at the standard, the semantic of salen is not specified. In doubt, I would consider the usage to be invalid even if it works by happenstance on Linux. > --- > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > index c6292c5..15fe25d 100644 > --- a/libavformat/rtsp.c > +++ b/libavformat/rtsp.c > @@ -2310,7 +2310,11 @@ static int sdp_read_header(AVFormatContext *s) > AVDictionary *opts = map_to_opts(rt); > > err = getnameinfo((struct sockaddr*) &rtsp_st->sdp_ip, > +#ifdef __FreeBSD__ > + ((struct sockaddr*) &rtsp_st->sdp_ip)->sa_len, > +#else > sizeof(rtsp_st->sdp_ip), > +#endif On the other hand, sa_len is not standard, and littering the code with ifdefry is ugly. Better add a field sdp_ip_len and set it at the same time as sdp_ip. Also, there are other instance of the same misuse of getnameinfo() in this file. > namebuf, sizeof(namebuf), NULL, 0, NI_NUMERICHOST); > if (err) { > av_log(s, AV_LOG_ERROR, "getnameinfo: %s\n", gai_strerror(err)); Regards,
On Thu, Nov 24, 2016 at 04:12:41PM +0100, Nicolas George wrote: > Le quartidi 4 frimaire, an CCXXV, Kevin Lo a écrit : > > FreeBSD's socket calls require the sockaddr struct length to agree > > with the address family, Linux does not. This patch fixes a failing > > getnameinfo() call on FreeBSD. > > > > Signed-off-by: Kevin Lo <kevlo@kevlo.org> > > I looked at the standard, the semantic of salen is not specified. In > doubt, I would consider the usage to be invalid even if it works by > happenstance on Linux. I know that's not the standard, that's why I added 'ifdef __FreeBSD__'. > > --- > > > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > > index c6292c5..15fe25d 100644 > > --- a/libavformat/rtsp.c > > +++ b/libavformat/rtsp.c > > @@ -2310,7 +2310,11 @@ static int sdp_read_header(AVFormatContext *s) > > AVDictionary *opts = map_to_opts(rt); > > > > err = getnameinfo((struct sockaddr*) &rtsp_st->sdp_ip, > > > +#ifdef __FreeBSD__ > > + ((struct sockaddr*) &rtsp_st->sdp_ip)->sa_len, > > +#else > > sizeof(rtsp_st->sdp_ip), > > +#endif > > On the other hand, sa_len is not standard, and littering the code with > ifdefry is ugly. Better add a field sdp_ip_len and set it at the same > time as sdp_ip. > > Also, there are other instance of the same misuse of getnameinfo() in > this file. > > > namebuf, sizeof(namebuf), NULL, 0, NI_NUMERICHOST); > > if (err) { > > av_log(s, AV_LOG_ERROR, "getnameinfo: %s\n", gai_strerror(err)); > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Fri, Nov 25, 2016 at 07:13:50AM +0800, Kevin Lo wrote: > > On Thu, Nov 24, 2016 at 04:12:41PM +0100, Nicolas George wrote: > > Le quartidi 4 frimaire, an CCXXV, Kevin Lo a écrit : > > > FreeBSD's socket calls require the sockaddr struct length to agree > > > with the address family, Linux does not. This patch fixes a failing > > > getnameinfo() call on FreeBSD. > > > > > > Signed-off-by: Kevin Lo <kevlo@kevlo.org> > > > > I looked at the standard, the semantic of salen is not specified. In > > doubt, I would consider the usage to be invalid even if it works by > > happenstance on Linux. > > I know that's not the standard, that's why I added 'ifdef __FreeBSD__'. Sorry I forgot to mention that I'll resend the patch about adding sdp_ip_len, thanks. > > > --- > > > > > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > > > index c6292c5..15fe25d 100644 > > > --- a/libavformat/rtsp.c > > > +++ b/libavformat/rtsp.c > > > @@ -2310,7 +2310,11 @@ static int sdp_read_header(AVFormatContext *s) > > > AVDictionary *opts = map_to_opts(rt); > > > > > > err = getnameinfo((struct sockaddr*) &rtsp_st->sdp_ip, > > > > > +#ifdef __FreeBSD__ > > > + ((struct sockaddr*) &rtsp_st->sdp_ip)->sa_len, > > > +#else > > > sizeof(rtsp_st->sdp_ip), > > > +#endif > > > > On the other hand, sa_len is not standard, and littering the code with > > ifdefry is ugly. Better add a field sdp_ip_len and set it at the same > > time as sdp_ip. > > > > Also, there are other instance of the same misuse of getnameinfo() in > > this file. > > > > > namebuf, sizeof(namebuf), NULL, 0, NI_NUMERICHOST); > > > if (err) { > > > av_log(s, AV_LOG_ERROR, "getnameinfo: %s\n", gai_strerror(err)); > > > > Regards, > > > > -- > > Nicolas George > > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index c6292c5..15fe25d 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -2310,7 +2310,11 @@ static int sdp_read_header(AVFormatContext *s) AVDictionary *opts = map_to_opts(rt); err = getnameinfo((struct sockaddr*) &rtsp_st->sdp_ip, +#ifdef __FreeBSD__ + ((struct sockaddr*) &rtsp_st->sdp_ip)->sa_len, +#else sizeof(rtsp_st->sdp_ip), +#endif namebuf, sizeof(namebuf), NULL, 0, NI_NUMERICHOST); if (err) { av_log(s, AV_LOG_ERROR, "getnameinfo: %s\n", gai_strerror(err));
FreeBSD's socket calls require the sockaddr struct length to agree with the address family, Linux does not. This patch fixes a failing getnameinfo() call on FreeBSD. Signed-off-by: Kevin Lo <kevlo@kevlo.org> ---