diff mbox series

[OpenWrt-Devel] libpcap: patch to add limits.h to pcap-usb-linux.c

Message ID mailman.13975.1533034632.25356.openwrt-devel@lists.openwrt.org
State Changes Requested
Delegated to: John Crispin
Headers show
Series [OpenWrt-Devel] libpcap: patch to add limits.h to pcap-usb-linux.c | expand

Commit Message

Thomas Richard via openwrt-devel July 31, 2018, 10:57 a.m. UTC
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.
Upstream patch that fixes compilation errors: 'PATH_MAX' and 'NAME_MAX'
undeclared with musl.

Signed-off-by: Eneas U de Queiroz <cote2004-github@yahoo.com>
---
 ...205-pcap-usb-linux.c-add-missing-limits.h.patch | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 package/libs/libpcap/patches/205-pcap-usb-linux.c-add-missing-limits.h.patch

Comments

John Crispin Aug. 1, 2018, 9:01 a.m. UTC | #1
none of the buildservers seem to break without this patch. please repost 
explaining how the build breaks without this patch. also reference the 
place where you copied the patch from
     John
Bjørn Mork Aug. 1, 2018, 12:08 p.m. UTC | #2
John Crispin <john@phrozen.org> writes:

> none of the buildservers seem to break without this patch. please
> repost explaining how the build breaks without this patch. also
> reference the place where you copied the patch from

The buildservers probably don't build this file since
CONFIG_PCAP_HAS_USB is 'n' by default?

Building on current master with

 bjorn@canardo:/usr/local/src/lede$ grep PCAP .config
 CONFIG_PCAP_HAS_USB=y
 CONFIG_PCAP_HAS_NETFILTER=y

results in:

bjorn@canardo:/usr/local/src/lede$ make V=s package/libpcap/compile
..
arm-openwrt-linux-muslgnueabi-gcc -Os -pipe -mcpu=cortex-a9 -mfpu=vfpv3-d16 -fno-caller-saves -fno-plt -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -mfloat-abi=hard -iremap/mnt/storage/media/.diverse/src/lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/libpcap-1.9.0:libpcap-1.9.0 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -ffunction-sections -fdata-sections -I/mnt/storage/media/.diverse/src/lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/linux/include -fpic -I.  -DBUILDING_PCAP -D_BSD_SOURCE -Dpcap_EXPORTS -DHAVE_CONFIG_H  -Os -pipe -mcpu=cortex-a9 -mfpu=vfpv3-d16 -fno-caller-saves -fno-plt -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -mfloat-abi=hard -iremap/mnt/storage/media/.diverse/src/lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/libpcap-1.9.0:libpcap-1.9.0 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -ffunction-sections -fdata-sections      -c -o pcap-usb-linux.o ./pcap-usb-linux.c
./pcap-usb-linux.c: In function 'usb_findalldevs':
./pcap-usb-linux.c:264:19: error: 'PATH_MAX' undeclared (first use in this function); did you mean 'AF_MAX'?
  char usb_mon_dir[PATH_MAX];
                   ^~~~~~~~
                   AF_MAX
./pcap-usb-linux.c:264:19: note: each undeclared identifier is reported only once for each function it appears in
./pcap-usb-linux.c: In function 'probe_devices':
./pcap-usb-linux.c:419:41: error: 'NAME_MAX' undeclared (first use in this function); did you mean 'AF_MAX'?
  char buf[sizeof("/dev/bus/usb/000/") + NAME_MAX];
                                         ^~~~~~~~
                                         AF_MAX
Makefile:94: recipe for target 'pcap-usb-linux.o' failed
make[3]: *** [pcap-usb-linux.o] Error 1
make[3]: Leaving directory '/home/bjorn/tmp/tmp-lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/libpcap-1.9.0'
Makefile:101: recipe for target '/mnt/storage/media/.diverse/src/lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/libpcap-1.9.0/.built' failed
make[2]: *** [/mnt/storage/media/.diverse/src/lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/libpcap-1.9.0/.built] Error 2
make[2]: Leaving directory '/mnt/storage/media/.diverse/src/lede/package/libs/libpcap'
Command exited with non-zero status 2
time: package/libs/libpcap/compile#2.20#0.39#6.47
package/Makefile:107: recipe for target 'package/libs/libpcap/compile' failed
make[1]: *** [package/libs/libpcap/compile] Error 2
make[1]: Leaving directory '/mnt/storage/media/.diverse/src/lede'
/mnt/storage/media/.diverse/src/lede/include/toplevel.mk:216: recipe for target 'package/libpcap/compile' failed
make: *** [package/libpcap/compile] Error 2


Patch looks good to me.




Bjørn
John Crispin Aug. 1, 2018, 12:11 p.m. UTC | #3
On 01/08/18 14:08, Bjørn Mork wrote:
> John Crispin <john@phrozen.org> writes:
>
>> none of the buildservers seem to break without this patch. please
>> repost explaining how the build breaks without this patch. also
>> reference the place where you copied the patch from
> The buildservers probably don't build this file since
> CONFIG_PCAP_HAS_USB is 'n' by default?
>
> Building on current master with
>
>   bjorn@canardo:/usr/local/src/lede$ grep PCAP .config
>   CONFIG_PCAP_HAS_USB=y
>   CONFIG_PCAP_HAS_NETFILTER=y
>
> results in:
>
> bjorn@canardo:/usr/local/src/lede$ make V=s package/libpcap/compile
> ..
> arm-openwrt-linux-muslgnueabi-gcc -Os -pipe -mcpu=cortex-a9 -mfpu=vfpv3-d16 -fno-caller-saves -fno-plt -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -mfloat-abi=hard -iremap/mnt/storage/media/.diverse/src/lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/libpcap-1.9.0:libpcap-1.9.0 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -ffunction-sections -fdata-sections -I/mnt/storage/media/.diverse/src/lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/linux/include -fpic -I.  -DBUILDING_PCAP -D_BSD_SOURCE -Dpcap_EXPORTS -DHAVE_CONFIG_H  -Os -pipe -mcpu=cortex-a9 -mfpu=vfpv3-d16 -fno-caller-saves -fno-plt -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -mfloat-abi=hard -iremap/mnt/storage/media/.diverse/src/lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/libpcap-1.9.0:libpcap-1.9.0 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -ffunction-sections -fdata-sections      -c -o pcap-usb-linux.o ./pcap-usb-linux.c
> ./pcap-usb-linux.c: In function 'usb_findalldevs':
> ./pcap-usb-linux.c:264:19: error: 'PATH_MAX' undeclared (first use in this function); did you mean 'AF_MAX'?
>    char usb_mon_dir[PATH_MAX];
>                     ^~~~~~~~
>                     AF_MAX
> ./pcap-usb-linux.c:264:19: note: each undeclared identifier is reported only once for each function it appears in
> ./pcap-usb-linux.c: In function 'probe_devices':
> ./pcap-usb-linux.c:419:41: error: 'NAME_MAX' undeclared (first use in this function); did you mean 'AF_MAX'?
>    char buf[sizeof("/dev/bus/usb/000/") + NAME_MAX];
>                                           ^~~~~~~~
>                                           AF_MAX
> Makefile:94: recipe for target 'pcap-usb-linux.o' failed
> make[3]: *** [pcap-usb-linux.o] Error 1
> make[3]: Leaving directory '/home/bjorn/tmp/tmp-lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/libpcap-1.9.0'
> Makefile:101: recipe for target '/mnt/storage/media/.diverse/src/lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/libpcap-1.9.0/.built' failed
> make[2]: *** [/mnt/storage/media/.diverse/src/lede/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/libpcap-1.9.0/.built] Error 2
> make[2]: Leaving directory '/mnt/storage/media/.diverse/src/lede/package/libs/libpcap'
> Command exited with non-zero status 2
> time: package/libs/libpcap/compile#2.20#0.39#6.47
> package/Makefile:107: recipe for target 'package/libs/libpcap/compile' failed
> make[1]: *** [package/libs/libpcap/compile] Error 2
> make[1]: Leaving directory '/mnt/storage/media/.diverse/src/lede'
> /mnt/storage/media/.diverse/src/lede/include/toplevel.mk:216: recipe for target 'package/libpcap/compile' failed
> make: *** [package/libpcap/compile] Error 2
>
>
> Patch looks good to me.
>
>
>
>
> Bjørn
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Hi Bjørn,

thanks for testing, I;ll merge it in a sec

	John
Thomas Richard via openwrt-devel Aug. 20, 2018, 10:42 a.m. UTC | #4
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.
I will repost the patch with a cover letter explaining all this, and with a better commit message.  I made the wrong assumption that the commit id on top of the patch would be enough to identify its origin.  I'm still learning, and shall do it right next time.

The reason why the buildservers don't fail is that they're probably not setting CONFIG_PCAP_HAS_USB (default is OFF), so the patched file does not get compiled.  I will add that info in the cover letter.

Cheers,
Eneas
    Em quarta-feira, 1 de agosto de 2018 06:01:58 BRT, John Crispin <john@phrozen.org> escreveu:  
 
 none of the buildservers seem to break without this patch. please repost 
explaining how the build breaks without this patch. also reference the 
place where you copied the patch from
     John
<html><head></head><body><div style="font-family:Helvetica Neue, Helvetica, Arial, sans-serif;font-size:13px;"><div style="font-family:Helvetica Neue, Helvetica, Arial, sans-serif;font-size:13px;"><div style="font-family:Helvetica Neue, Helvetica, Arial, sans-serif;font-size:13px;"><div></div>
        <div>I will repost the patch with a cover letter explaining all this, and with a better commit message.&nbsp; I made the wrong assumption that the commit id on top of the patch would be enough to identify its origin.&nbsp; I'm still learning, and shall do it right next time.<br></div><div><br></div><div>The reason why the buildservers don't fail is that they're probably not setting<span><span> CONFIG_PCAP_HAS_USB (default is OFF), so the patched file does not get compiled.&nbsp; I will add that info in the cover letter.<br></span></span></div><div><br><span><span></span></span></div><div>Cheers,</div><div><br></div><div>Eneas</div><div><br></div>
        
        </div></div><div id="ydp2bd1deb5yahoo_quoted_3947069750" class="ydp2bd1deb5yahoo_quoted">
            <div style="font-family:'Helvetica Neue', Helvetica, Arial, sans-serif;font-size:13px;color:#26282a;">
                
                <div>
                    Em quarta-feira, 1 de agosto de 2018 06:01:58 BRT, John Crispin &lt;john@phrozen.org&gt; escreveu:
                </div>
                <div><br></div>
                <div><br></div>
                <div><div dir="ltr">none of the buildservers seem to break without this patch. please repost <br></div><div dir="ltr">explaining how the build breaks without this patch. also reference the <br></div><div dir="ltr">place where you copied the patch from<br></div><div dir="ltr"> &nbsp;&nbsp;&nbsp; John<br></div></div>
            </div>
        </div></div></body></html>
Bjørn Mork Aug. 20, 2018, 12:07 p.m. UTC | #5
Eneas Ulir de Queiroz via openwrt-devel
<openwrt-devel@lists.openwrt.org> writes:

> The reason why the buildservers don't fail is that they're probably
> not setting CONFIG_PCAP_HAS_USB (default is OFF), so the patched file
> does not get compiled.  I will add that info in the cover letter.

Not directly related.... But I wonder if maybe this default could be
changed?

I realize that this is a space thing, but having an optional feature
defaulting to off means that practically nobody uses the feature.  And
this one is particularily useful.  At least to me :-) Which of course is
no argument since I can easily build my own images with the feature
enabled.

The argument for changing the default is that it would make life a hell
of a lot easier for anyone doing USB driver development, saving quite a
few round-trip times trying to get USB captures from ordinary OpenWrt
end users. So I'd like to see CONFIG_PCAP_HAS_USB enabled
unconditionally, and rather have an optional libpcap-mini package with
less features if necessary.  Which I am not convinced it is....


Bjørn
Michael Richardson Aug. 20, 2018, 2:21 p.m. UTC | #6
Bjørn Mork <bjorn@mork.no> wrote:
    >> The reason why the buildservers don't fail is that they're probably
    >> not setting CONFIG_PCAP_HAS_USB (default is OFF), so the patched file
    >> does not get compiled.  I will add that info in the cover letter.

    > Not directly related.... But I wonder if maybe this default could be
    > changed?

    > I realize that this is a space thing, but having an optional feature
    > defaulting to off means that practically nobody uses the feature.  And
    > this one is particularily useful.  At least to me :-) Which of course
    > is no argument since I can easily build my own images with the feature
    > enabled.

I also agree, but turning in USB requires additional libraries, and I'm not
sure that many will need USB captures on openwrt.

I (as mcr@tcpdump.org) don't understand why you are seeing a build failure
that causes you to want to patch the file.

Also, the latest libpcap (1.9.0) has cmake support, which might make things
easier?

--
]               Never tell me the odds!                 | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works        | network architect  [
]     mcr@sandelman.ca  http://www.sandelman.ca/        |   ruby on rails    [
Bjørn Mork Aug. 20, 2018, 4:04 p.m. UTC | #7
Michael Richardson <mcr@sandelman.ca> writes:
> Bjørn Mork <bjorn@mork.no> wrote:
>     >> The reason why the buildservers don't fail is that they're probably
>     >> not setting CONFIG_PCAP_HAS_USB (default is OFF), so the patched file
>     >> does not get compiled.  I will add that info in the cover letter.
>
>     > Not directly related.... But I wonder if maybe this default could be
>     > changed?
>
>     > I realize that this is a space thing, but having an optional feature
>     > defaulting to off means that practically nobody uses the feature.  And
>     > this one is particularily useful.  At least to me :-) Which of course
>     > is no argument since I can easily build my own images with the feature
>     > enabled.
>
> I also agree, but turning in USB requires additional libraries, and I'm not
> sure that many will need USB captures on openwrt.

I agree that most users won't need it. The problem is the high threshold
for those few who do.

I've been into the LTE modem driver business for a while. Captures from
end users with some modem hardware/firmware specific issue have often
been the only way to fully understand a problem. Changing an option
hidden deep inside the OpenWrt build config is out of the question
unless the reporter already is familiar with that build system.  It is
about as convenient as a kernel debug patch for most end users.  The
lack of a pre-built usbmon capture tool makes bug reports from OpenWrt
users much harder to debug.

BTW: I tried to figure out which additional libraries the feature
requires.  It doesn't seem to be any? pcap-usb-linux.c uses the Linux
kernel support directly.  Or am I missing something?

> I (as mcr@tcpdump.org) don't understand why you are seeing a build failure
> that causes you to want to patch the file.

The build failure is in version 1.8.1 used by OpenWrt 18.06.  The fix
being discussed is a backport of this upstream patch:

 https://github.com/the-tcpdump-group/libpcap/commit/aafa3512b7b742f5e66a5543e41974cc5e7eebfa

> Also, the latest libpcap (1.9.0) has cmake support, which might make things
> easier?

Maybe? All these questions are of course up to the maintainer. Adding
Felix to the CC list.



Bjørn
Michael Richardson Aug. 20, 2018, 4:20 p.m. UTC | #8
Bjørn Mork <bjorn@mork.no> wrote:
    > I've been into the LTE modem driver business for a while. Captures from
    > end users with some modem hardware/firmware specific issue have often
    > been the only way to fully understand a problem. Changing an option
    > hidden deep inside the OpenWrt build config is out of the question
    > unless the reporter already is familiar with that build system.  It is
    > about as convenient as a kernel debug patch for most end users.  The
    > lack of a pre-built usbmon capture tool makes bug reports from OpenWrt
    > users much harder to debug.

That's definitely a reason to include USB capture then!

    > BTW: I tried to figure out which additional libraries the feature
    > requires.  It doesn't seem to be any? pcap-usb-linux.c uses the Linux
    > kernel support directly.  Or am I missing something?

It requires libusb.

    >> Also, the latest libpcap (1.9.0) has cmake support, which might make
    >> things easier?

    > Maybe? All these questions are of course up to the maintainer. Adding
    > Felix to the CC list.

Some love cmake, some hate it.
When it works, I find I like it.  but, determining how to tweak options is
really arcane.

--
]               Never tell me the odds!                 | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works        | network architect  [
]     mcr@sandelman.ca  http://www.sandelman.ca/        |   ruby on rails    [
diff mbox series

Patch

diff --git a/package/libs/libpcap/patches/205-pcap-usb-linux.c-add-missing-limits.h.patch b/package/libs/libpcap/patches/205-pcap-usb-linux.c-add-missing-limits.h.patch
new file mode 100644
index 0000000000..cae8012ede
--- /dev/null
+++ b/package/libs/libpcap/patches/205-pcap-usb-linux.c-add-missing-limits.h.patch
@@ -0,0 +1,22 @@ 
+From aafa3512b7b742f5e66a5543e41974cc5e7eebfa Mon Sep 17 00:00:00 2001
+From: maxice8 <thinkabit.ukim@gmail.com>
+Date: Sun, 22 Jul 2018 18:54:17 -0300
+Subject: [PATCH] pcap-usb-linux.c: add missing limits.h for musl systems.
+
+fix compilation on musl libc systems like Void Linux and Alpine.
+---
+ pcap-usb-linux.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/pcap-usb-linux.c b/pcap-usb-linux.c
+index 6f8adf65e..b92c05ea1 100644
+--- a/pcap-usb-linux.c
++++ b/pcap-usb-linux.c
+@@ -50,6 +50,7 @@
+ #include <stdlib.h>
+ #include <unistd.h>
+ #include <fcntl.h>
++#include <limits.h>
+ #include <string.h>
+ #include <dirent.h>
+ #include <byteswap.h>