Re: [FFmpeg-devel] [PATCH v2] avformat/rtsp: introduce get_sa_len() function

2016-11-25 Thread Nicolas George
> >Since the Linux implementation of sockaddr doesn't have sa_len as a member,
> >but the FreeBSD version does, introduce a get_sa_len() function that
> >determines the size based on the address family.
> >
> >Signed-off-by: Kevin Lo 

For some reason I do not have the original mail in my archives. I reply
to this one instead.

> >---
> >
> >diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> >index c6292c5..4c543ed 100644
> >--- a/libavformat/rtsp.c
> >+++ b/libavformat/rtsp.c
> >@@ -202,6 +202,19 @@ static int get_sockaddr(AVFormatContext *s,
> >  return 0;
> >  }
> >
> >+static socklen_t
> >+get_sa_len(struct sockaddr *addr)
> >+{
> >+switch (addr->sa_family) {
> >+case AF_INET:
> >+return (sizeof(struct sockaddr_in));
> >+case AF_INET6:
> >+return (sizeof(struct sockaddr_in6));
> >+default:
> >+return (sizeof(struct sockaddr));
> >+}

I think this is misdesigned. It will not work if new address families
are added, while the whole point of the modern socket API is to
transparently handle that.

The system always gives us the correct size of the sockaddr structure
(ai_addrlen in struct addrinfo, the third argument of getpeername(),
etc.), we only have to keep it instead of discarding it.

Regards,

-- 
  Nicolas George
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v2] avformat/rtsp: introduce get_sa_len() function

2016-11-24 Thread Kevin Lo
On Thu, Nov 24, 2016 at 10:48:38PM -0800, Dave Yeo wrote:
> 
> On 11/24/16 07:47 PM, Kevin Lo wrote:
> > Since the Linux implementation of sockaddr doesn't have sa_len as a member,
> > but the FreeBSD version does, introduce a get_sa_len() function that
> > determines the size based on the address family.
> >
> > Signed-off-by: Kevin Lo 
> > ---
> >
> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > index c6292c5..4c543ed 100644
> > --- a/libavformat/rtsp.c
> > +++ b/libavformat/rtsp.c
> > @@ -202,6 +202,19 @@ static int get_sockaddr(AVFormatContext *s,
> >   return 0;
> >   }
> >
> > +static socklen_t
> > +get_sa_len(struct sockaddr *addr)
> > +{
> > +switch (addr->sa_family) {
> > +case AF_INET:
> > +   return (sizeof(struct sockaddr_in));
> > +case AF_INET6:
> > +   return (sizeof(struct sockaddr_in6));
> > +default:
> > +   return (sizeof(struct sockaddr));
> > +}
> > +}
> > +
> [...]
> 
> Fails here (OS/2),
> ...
> CC  libavformat/rtspdec.o
> K:/usr/local/src/ffmpeg/libavformat/rtsp.c: In function 'get_sa_len':
> K:/usr/local/src/ffmpeg/libavformat/rtsp.c:212:17: error: invalid 
> application of
>   'sizeof' to incomplete type 'struct sockaddr_in6'
>return (sizeof(struct sockaddr_in6));
>   ^
> make: *** [libavformat/rtsp.o] Error 1
> ...
> Perhaps use
> #if HAVE_STRUCT_SOCKADDR_IN6
>  case AF_INET6:
>  return (sizeof(struct sockaddr_in6));
> #endif
> or such.

Thanks for pointing it out, I just resent the patch, thanks

> Dave

Kevin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v2] avformat/rtsp: introduce get_sa_len() function

2016-11-24 Thread Dave Yeo

On 11/24/16 07:47 PM, Kevin Lo wrote:

Since the Linux implementation of sockaddr doesn't have sa_len as a member,
but the FreeBSD version does, introduce a get_sa_len() function that
determines the size based on the address family.

Signed-off-by: Kevin Lo 
---

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index c6292c5..4c543ed 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -202,6 +202,19 @@ static int get_sockaddr(AVFormatContext *s,
  return 0;
  }

+static socklen_t
+get_sa_len(struct sockaddr *addr)
+{
+switch (addr->sa_family) {
+case AF_INET:
+   return (sizeof(struct sockaddr_in));
+case AF_INET6:
+   return (sizeof(struct sockaddr_in6));
+default:
+   return (sizeof(struct sockaddr));
+}
+}
+

[...]

Fails here (OS/2),
...
CC  libavformat/rtspdec.o
K:/usr/local/src/ffmpeg/libavformat/rtsp.c: In function 'get_sa_len':
K:/usr/local/src/ffmpeg/libavformat/rtsp.c:212:17: error: invalid 
application of

 'sizeof' to incomplete type 'struct sockaddr_in6'
  return (sizeof(struct sockaddr_in6));
 ^
make: *** [libavformat/rtsp.o] Error 1
...
Perhaps use
#if HAVE_STRUCT_SOCKADDR_IN6
case AF_INET6:
return (sizeof(struct sockaddr_in6));
#endif
or such.

Dave
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2] avformat/rtsp: introduce get_sa_len() function

2016-11-24 Thread Kevin Lo
Since the Linux implementation of sockaddr doesn't have sa_len as a member,
but the FreeBSD version does, introduce a get_sa_len() function that
determines the size based on the address family.

Signed-off-by: Kevin Lo 
---

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index c6292c5..4c543ed 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -202,6 +202,19 @@ static int get_sockaddr(AVFormatContext *s,
 return 0;
 }
 
+static socklen_t
+get_sa_len(struct sockaddr *addr)
+{
+switch (addr->sa_family) {
+case AF_INET:
+   return (sizeof(struct sockaddr_in));
+case AF_INET6:
+   return (sizeof(struct sockaddr_in6));
+default:
+   return (sizeof(struct sockaddr));
+}
+}
+
 #if CONFIG_RTPDEC
 static void init_rtp_handler(RTPDynamicProtocolHandler *handler,
  RTSPStream *rtsp_st, AVStream *st)
@@ -1614,7 +1627,8 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const 
char *host, int port,
 }
 if (ttl > 0)
 snprintf(optbuf, sizeof(optbuf), "?ttl=%d", ttl);
-getnameinfo((struct sockaddr*) , sizeof(addr),
+getnameinfo((struct sockaddr*) ,
+   get_sa_len((struct sockaddr*) ),
 namebuf, sizeof(namebuf), NULL, 0, NI_NUMERICHOST);
 ff_url_join(url, sizeof(url), "rtp", NULL, namebuf,
 port, "%s", optbuf);
@@ -1830,7 +1844,8 @@ redirect:
 goto fail;
 }
 if (!getpeername(tcp_fd, (struct sockaddr*) , _len)) {
-getnameinfo((struct sockaddr*) , peer_len, host, sizeof(host),
+getnameinfo((struct sockaddr*) ,
+   get_sa_len((struct sockaddr*) ), host, sizeof(host),
 NULL, 0, NI_NUMERICHOST);
 }
 
@@ -2310,7 +2325,7 @@ static int sdp_read_header(AVFormatContext *s)
 AVDictionary *opts = map_to_opts(rt);
 
 err = getnameinfo((struct sockaddr*) _st->sdp_ip,
-  sizeof(rtsp_st->sdp_ip),
+  get_sa_len((struct sockaddr*) _st->sdp_ip),
   namebuf, sizeof(namebuf), NULL, 0, 
NI_NUMERICHOST);
 if (err) {
 av_log(s, AV_LOG_ERROR, "getnameinfo: %s\n", 
gai_strerror(err));
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel