| Message ID | f4740c20-da0f-4f98-87ab-79407d41e7ef@amyspark.me |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] os_support, network: Fix build failure on Windows with BZIP2 | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/configure_loongarch64 | warning | Failed to apply patch |
| andriy/configure_x86 | warning | Failed to apply patch |
On Thu, 27 Jul 2023, L. E. Segovia wrote: > Including winsock2.h without WIN32_LEAN_AND_MEAN causes bzlib.h to parse > as nonsense, due to an instance of #define char small in rpcndr.h > (included transitively from windows.h). > > See: https://stackoverflow.com/a/27794577 > Signed-off-by: L. E. Segovia <amy@amyspark.me> > --- > libavformat/network.h | 1 + > libavformat/os_support.c | 6 ++---- > libavformat/os_support.h | 1 + > 3 files changed, 4 insertions(+), 4 deletions(-) The change looks mostly reasonable to me I think, and WIN32_LEAN_AND_MEAN is generally beneficial. I've got a couple comments below though. > diff --git a/libavformat/network.h b/libavformat/network.h > index ca214087fc..06b6117fc7 100644 > --- a/libavformat/network.h > +++ b/libavformat/network.h > @@ -35,6 +35,7 @@ > #endif > #if HAVE_WINSOCK2_H > +#define WIN32_LEAN_AND_MEAN > #include <winsock2.h> > #include <ws2tcpip.h> The diff seems very hard to apply. The diff stat above says that this shows a snippet of 6 lines originally, 7 lines after the modification - but in fact the diff only shows 4 lines originally and 5 lines after the modification. I don't know what has happened to the patch, but it makes it hard to apply automatically. > diff --git a/libavformat/os_support.c b/libavformat/os_support.c > index 15cea7fa5b..2de6a7c3d9 100644 > --- a/libavformat/os_support.c > +++ b/libavformat/os_support.c > @@ -34,11 +34,9 @@ > #if HAVE_SYS_TIME_H > #include <sys/time.h> > #endif /* HAVE_SYS_TIME_H */ > -#if HAVE_WINSOCK2_H > -#include <winsock2.h> > -#elif HAVE_SYS_SELECT_H > +#if HAVE_SYS_SELECT_H > #include <sys/select.h> > -#endif /* HAVE_WINSOCK2_H */ > +#endif /* HAVE_SYS_SELECT_H */ > #endif /* !HAVE_POLL_H */ > #include "network.h" I presume this is done to avoid touching winsock2.h here, as the headers that we've included already define this? // Martin
On Fri, 4 Aug 2023, Martin Storsjö wrote: > On Thu, 27 Jul 2023, L. E. Segovia wrote: > >> Including winsock2.h without WIN32_LEAN_AND_MEAN causes bzlib.h to parse >> as nonsense, due to an instance of #define char small in rpcndr.h >> (included transitively from windows.h). >> >> See: https://stackoverflow.com/a/27794577 >> Signed-off-by: L. E. Segovia <amy@amyspark.me> >> --- >> libavformat/network.h | 1 + >> libavformat/os_support.c | 6 ++---- >> libavformat/os_support.h | 1 + >> 3 files changed, 4 insertions(+), 4 deletions(-) > > The change looks mostly reasonable to me I think, and WIN32_LEAN_AND_MEAN is > generally beneficial. I've got a couple comments below though. Alternatively, I guess we could consider adding WIN32_LEAN_AND_MEAN in configure somewhere instead? That way we don't need to hunt down any potential stray includes of windows.h/winsock2.h if they are added elsewhere. // Martin
Hey, Yes, doing so directly in the configure script could be more useful. In fact, while I was doing further internal testing, I found two more instances that needed patching up. I'll update this set and let you know. amyspark On 04/08/2023 09:39, Martin Storsjö wrote: > On Fri, 4 Aug 2023, Martin Storsjö wrote: > >> On Thu, 27 Jul 2023, L. E. Segovia wrote: >> >>> Including winsock2.h without WIN32_LEAN_AND_MEAN causes bzlib.h to parse >>> as nonsense, due to an instance of #define char small in rpcndr.h >>> (included transitively from windows.h). >>> >>> See: https://stackoverflow.com/a/27794577 >>> Signed-off-by: L. E. Segovia <amy@amyspark.me> >>> --- >>> libavformat/network.h | 1 + >>> libavformat/os_support.c | 6 ++---- >>> libavformat/os_support.h | 1 + >>> 3 files changed, 4 insertions(+), 4 deletions(-) >> >> The change looks mostly reasonable to me I think, and WIN32_LEAN_AND_MEAN is generally beneficial. I've got a couple comments below though. > > Alternatively, I guess we could consider adding WIN32_LEAN_AND_MEAN in configure somewhere instead? That way we don't need to hunt down any potential stray includes of windows.h/winsock2.h if they are added elsewhere. > > // Martin > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavformat/network.h b/libavformat/network.h index ca214087fc..06b6117fc7 100644 --- a/libavformat/network.h +++ b/libavformat/network.h @@ -35,6 +35,7 @@ #endif #if HAVE_WINSOCK2_H +#define WIN32_LEAN_AND_MEAN #include <winsock2.h> #include <ws2tcpip.h> diff --git a/libavformat/os_support.c b/libavformat/os_support.c index 15cea7fa5b..2de6a7c3d9 100644 --- a/libavformat/os_support.c +++ b/libavformat/os_support.c @@ -34,11 +34,9 @@ #if HAVE_SYS_TIME_H #include <sys/time.h> #endif /* HAVE_SYS_TIME_H */ -#if HAVE_WINSOCK2_H -#include <winsock2.h> -#elif HAVE_SYS_SELECT_H +#if HAVE_SYS_SELECT_H #include <sys/select.h> -#endif /* HAVE_WINSOCK2_H */ +#endif /* HAVE_SYS_SELECT_H */ #endif /* !HAVE_POLL_H */ #include "network.h" diff --git a/libavformat/os_support.h b/libavformat/os_support.h index f2ff38e23b..5bdd275d70 100644 --- a/libavformat/os_support.h +++ b/libavformat/os_support.h @@ -140,6 +140,7 @@ typedef int socklen_t; typedef unsigned long nfds_t; #if HAVE_WINSOCK2_H +#define WIN32_LEAN_AND_MEAN #include <winsock2.h> #endif #if !HAVE_STRUCT_POLLFD -- 2.41.0
Including winsock2.h without WIN32_LEAN_AND_MEAN causes bzlib.h to parse as nonsense, due to an instance of #define char small in rpcndr.h (included transitively from windows.h). See: https://stackoverflow.com/a/27794577 Signed-off-by: L. E. Segovia <amy@amyspark.me> --- libavformat/network.h | 1 + libavformat/os_support.c | 6 ++---- libavformat/os_support.h | 1 + 3 files changed, 4 insertions(+), 4 deletions(-)