Re: [FFmpeg-devel] [PATCH 1/7] avformat/ip: factorize some IP filtering and resolving functions to a new file
On Mon, Sep 24, 2018 at 12:04:12AM +0200, Marton Balint wrote: > > > On Sun, 23 Sep 2018, Michael Niedermayer wrote: > > >On Sat, Sep 22, 2018 at 11:53:22PM +0200, Marton Balint wrote: > >>These are based on the very similar UDP and RTP protocol functions. > >> > >>Signed-off-by: Marton Balint > >>--- > >> libavformat/ip.c | 165 > >> +++ > >> libavformat/ip.h | 74 + > >> 2 files changed, 239 insertions(+) > >> create mode 100644 libavformat/ip.c > >> create mode 100644 libavformat/ip.h > >[...] > >>+/** > >>+ * Parses the address[,address] source list in buf and adds it to the > >>filters > >>+ * in the IPSourceFilters structure. > >>+ * @param buf is the source list, which is not changed, but must be > >>writable > >>+ * @return 0 on success, < 0 AVERROR code on error. > >>+ */ > >>+int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters > >>*filters); > >>+ > >>+/** > >>+ * Parses the address[,address] source block list in buf and adds it to the > >>+ * filters in the IPSourceFilters structure. > >>+ * @param buf is the source list, which is not changed, but must be > >>writable > > > >if its not changed, why does it need to be writable ? > > Becasue the during the parsing the ',' is replaced with '\0' temporarily. this implies that the buffer cannot be shared with another thread. (which while unlikely could if its done result in very hard to reproduce bugs) This should be at least documented more clearly but i think it would be best to avoid these temporary writes as they are unexpected. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/7] avformat/ip: factorize some IP filtering and resolving functions to a new file
On Sun, 23 Sep 2018, James Almer wrote: On 9/23/2018 7:04 PM, Marton Balint wrote: On Sun, 23 Sep 2018, Michael Niedermayer wrote: On Sat, Sep 22, 2018 at 11:53:22PM +0200, Marton Balint wrote: These are based on the very similar UDP and RTP protocol functions. Signed-off-by: Marton Balint --- libavformat/ip.c | 165 +++ libavformat/ip.h | 74 + 2 files changed, 239 insertions(+) create mode 100644 libavformat/ip.c create mode 100644 libavformat/ip.h [...] +/** + * Parses the address[,address] source list in buf and adds it to the filters + * in the IPSourceFilters structure. + * @param buf is the source list, which is not changed, but must be writable + * @return 0 on success, < 0 AVERROR code on error. + */ +int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters *filters); + +/** + * Parses the address[,address] source block list in buf and adds it to the + * filters in the IPSourceFilters structure. + * @param buf is the source list, which is not changed, but must be writable if its not changed, why does it need to be writable ? Becasue the during the parsing the ',' is replaced with '\0' temporarily. Why not use av_get_token() or similar instead, to split it into separate strings? That way the buffer passed to the function can be const. That would cause some unnecesary mallocs/frees because we don't store the parsed address strings. Also this patch is mostly factorization, I tried to keep functional changes to the mimnium... Can be changed if any of you still prefer that. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/7] avformat/ip: factorize some IP filtering and resolving functions to a new file
On 9/23/2018 7:04 PM, Marton Balint wrote: > > > On Sun, 23 Sep 2018, Michael Niedermayer wrote: > >> On Sat, Sep 22, 2018 at 11:53:22PM +0200, Marton Balint wrote: >>> These are based on the very similar UDP and RTP protocol functions. >>> >>> Signed-off-by: Marton Balint >>> --- >>> libavformat/ip.c | 165 >>> +++ >>> libavformat/ip.h | 74 + >>> 2 files changed, 239 insertions(+) >>> create mode 100644 libavformat/ip.c >>> create mode 100644 libavformat/ip.h >> [...] >>> +/** >>> + * Parses the address[,address] source list in buf and adds it to >>> the filters >>> + * in the IPSourceFilters structure. >>> + * @param buf is the source list, which is not changed, but must be >>> writable >>> + * @return 0 on success, < 0 AVERROR code on error. >>> + */ >>> +int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters >>> *filters); >>> + >>> +/** >>> + * Parses the address[,address] source block list in buf and adds it >>> to the >>> + * filters in the IPSourceFilters structure. >>> + * @param buf is the source list, which is not changed, but must be >>> writable >> >> if its not changed, why does it need to be writable ? > > Becasue the during the parsing the ',' is replaced with '\0' temporarily. Why not use av_get_token() or similar instead, to split it into separate strings? That way the buffer passed to the function can be const. > > Regards, > Marton > ___ > 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
Re: [FFmpeg-devel] [PATCH 1/7] avformat/ip: factorize some IP filtering and resolving functions to a new file
On Sun, 23 Sep 2018, Michael Niedermayer wrote: On Sat, Sep 22, 2018 at 11:53:22PM +0200, Marton Balint wrote: These are based on the very similar UDP and RTP protocol functions. Signed-off-by: Marton Balint --- libavformat/ip.c | 165 +++ libavformat/ip.h | 74 + 2 files changed, 239 insertions(+) create mode 100644 libavformat/ip.c create mode 100644 libavformat/ip.h [...] +/** + * Parses the address[,address] source list in buf and adds it to the filters + * in the IPSourceFilters structure. + * @param buf is the source list, which is not changed, but must be writable + * @return 0 on success, < 0 AVERROR code on error. + */ +int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters *filters); + +/** + * Parses the address[,address] source block list in buf and adds it to the + * filters in the IPSourceFilters structure. + * @param buf is the source list, which is not changed, but must be writable if its not changed, why does it need to be writable ? Becasue the during the parsing the ',' is replaced with '\0' temporarily. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/7] avformat/ip: factorize some IP filtering and resolving functions to a new file
On Sat, Sep 22, 2018 at 11:53:22PM +0200, Marton Balint wrote: > These are based on the very similar UDP and RTP protocol functions. > > Signed-off-by: Marton Balint > --- > libavformat/ip.c | 165 > +++ > libavformat/ip.h | 74 + > 2 files changed, 239 insertions(+) > create mode 100644 libavformat/ip.c > create mode 100644 libavformat/ip.h [...] > +/** > + * Parses the address[,address] source list in buf and adds it to the filters > + * in the IPSourceFilters structure. > + * @param buf is the source list, which is not changed, but must be writable > + * @return 0 on success, < 0 AVERROR code on error. > + */ > +int ff_ip_parse_sources(void *log_ctx, char *buf, IPSourceFilters *filters); > + > +/** > + * Parses the address[,address] source block list in buf and adds it to the > + * filters in the IPSourceFilters structure. > + * @param buf is the source list, which is not changed, but must be writable if its not changed, why does it need to be writable ? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User questions about the command line tools should be sent to the ffmpeg-user ML. And questions about how to use libav* should be sent to the libav-user ML. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/7] avformat/ip: factorize some IP filtering and resolving functions to a new file
These are based on the very similar UDP and RTP protocol functions.
Signed-off-by: Marton Balint
---
libavformat/ip.c | 165 +++
libavformat/ip.h | 74 +
2 files changed, 239 insertions(+)
create mode 100644 libavformat/ip.c
create mode 100644 libavformat/ip.h
diff --git a/libavformat/ip.c b/libavformat/ip.c
new file mode 100644
index 00..d88055ebda
--- /dev/null
+++ b/libavformat/ip.c
@@ -0,0 +1,165 @@
+/*
+ * IP common code
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with FFmpeg; if not, write to the Free Software * Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "ip.h"
+
+static int compare_addr(const struct sockaddr_storage *a,
+const struct sockaddr_storage *b)
+{
+if (a->ss_family != b->ss_family)
+return 1;
+if (a->ss_family == AF_INET) {
+return (((const struct sockaddr_in *)a)->sin_addr.s_addr !=
+((const struct sockaddr_in *)b)->sin_addr.s_addr);
+}
+
+#if HAVE_STRUCT_SOCKADDR_IN6
+if (a->ss_family == AF_INET6) {
+const uint8_t *s6_addr_a = ((const struct sockaddr_in6
*)a)->sin6_addr.s6_addr;
+const uint8_t *s6_addr_b = ((const struct sockaddr_in6
*)b)->sin6_addr.s6_addr;
+return memcmp(s6_addr_a, s6_addr_b, 16);
+}
+#endif
+return 1;
+}
+
+int ff_ip_check_source_lists(struct sockaddr_storage *source_addr_ptr,
IPSourceFilters *s)
+{
+int i;
+if (s->nb_exclude_addrs) {
+for (i = 0; i < s->nb_exclude_addrs; i++) {
+if (!compare_addr(source_addr_ptr, >exclude_addrs[i]))
+return 1;
+}
+}
+if (s->nb_include_addrs) {
+for (i = 0; i < s->nb_include_addrs; i++) {
+if (!compare_addr(source_addr_ptr, >include_addrs[i]))
+return 0;
+}
+return 1;
+}
+return 0;
+}
+
+struct addrinfo *ff_ip_resolve_host(void *log_ctx,
+const char *hostname, int port,
+int type, int family, int flags)
+{
+struct addrinfo hints = { 0 }, *res = 0;
+int error;
+char sport[16];
+const char *node = 0, *service = "0";
+
+if (port > 0) {
+snprintf(sport, sizeof(sport), "%d", port);
+service = sport;
+}
+if ((hostname) && (hostname[0] != '\0') && (hostname[0] != '?')) {
+node = hostname;
+}
+hints.ai_socktype = type;
+hints.ai_family = family;
+hints.ai_flags= flags;
+if ((error = getaddrinfo(node, service, , ))) {
+res = NULL;
+av_log(log_ctx, AV_LOG_ERROR, "getaddrinfo(%s, %s): %s\n",
+ node ? node : "unknown",
+ service,
+ gai_strerror(error));
+}
+
+return res;
+}
+
+
+static int ip_parse_addr_list(void *log_ctx, char *buf,
+ struct sockaddr_storage **address_list_ptr,
+ int *address_list_size_ptr)
+{
+struct addrinfo *ai = NULL;
+char tmp = '\0', *p = buf, *next;
+
+/* Resolve all of the IPs */
+
+while (p && p[0]) {
+next = strchr(p, ',');
+
+if (next) {
+tmp = *next;
+*next = '\0';
+}
+
+ai = ff_ip_resolve_host(log_ctx, p, 0, SOCK_DGRAM, AF_UNSPEC, 0);
+
+if (next) {
+*next = tmp;
+p = next + 1;
+} else {
+p = NULL;
+}
+
+if (ai) {
+struct sockaddr_storage source_addr = {0};
+memcpy(_addr, ai->ai_addr, ai->ai_addrlen);
+freeaddrinfo(ai);
+av_dynarray2_add((void **)address_list_ptr, address_list_size_ptr,
sizeof(source_addr), (uint8_t *)_addr);
+if (!*address_list_ptr)
+return AVERROR(ENOMEM);
+} else {
+return AVERROR(EINVAL);
+}
+}
+
+return 0;
+}
+
+static int ip_parse_sources_and_blocks(void *log_ctx, char *buf,
IPSourceFilters *filters, int parse_include_list)
+{
+int ret;
+if (parse_include_list)
+ret = ip_parse_addr_list(log_ctx, buf, >include_addrs,
>nb_include_addrs);
+else
+ret = ip_parse_addr_list(log_ctx, buf, >exclude_addrs,
>nb_exclude_addrs);
+
+if (ret >= 0 && filters->nb_include_addrs &&
