| Message ID | 20200714054351.116191-1-rosenp@gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | Petr Štetiar |
| Headers | show |
| Series | iftop: fix compilation with GCC 10 | expand |
Rosen Penev <rosenp@gmail.com> [2020-07-13 22:43:51]:
Hi,
> GCC 10 defaults to fno-common, which demains unique defenitions.
whats the exact error here? Wouldn't it make sense to move this into packages feed?
What about upstream, looks like they might find this patch handy.
-- ynezz
> On Jul 17, 2020, at 2:50 AM, Petr Štetiar <ynezz@true.cz> wrote: > > Rosen Penev <rosenp@gmail.com> [2020-07-13 22:43:51]: > > Hi, > >> GCC 10 defaults to fno-common, which demains unique defenitions. > > whats the exact error here? Wouldn't it make sense to move this into packages feed? No idea what this package is for or the reason for its presence here. > What about upstream, looks like they might find this patch handy. Seems relatively dead upstream. Last commit one year ago and second to last three years ago. > > -- ynezz
On 2020-07-14 07:43, Rosen Penev wrote: > GCC 10 defaults to fno-common, which demains unique defenitions. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > --- > package/network/utils/iftop/Makefile | 2 +- > .../utils/iftop/patches/010-gcc10.patch | 20 +++++++++++++++++++ > 2 files changed, 21 insertions(+), 1 deletion(-) > create mode 100644 package/network/utils/iftop/patches/010-gcc10.patch > > diff --git a/package/network/utils/iftop/Makefile b/package/network/utils/iftop/Makefile > index 98fe15c8f5..aa467c2876 100644 > --- a/package/network/utils/iftop/Makefile > +++ b/package/network/utils/iftop/Makefile > @@ -8,7 +8,7 @@ > include $(TOPDIR)/rules.mk > > PKG_NAME:=iftop > -PKG_RELEASE:=1 > +PKG_RELEASE:=2 > > PKG_SOURCE_PROTO:=git > PKG_SOURCE_URL:=https://code.blinkace.com/pdw/iftop.git > diff --git a/package/network/utils/iftop/patches/010-gcc10.patch b/package/network/utils/iftop/patches/010-gcc10.patch > new file mode 100644 > index 0000000000..882565a039 > --- /dev/null > +++ b/package/network/utils/iftop/patches/010-gcc10.patch > @@ -0,0 +1,20 @@ > +--- a/ui_common.h > ++++ b/ui_common.h > +@@ -33,12 +33,12 @@ typedef struct host_pair_line_tag { > + > + extern options_t options; > + > +-sorted_list_type screen_list; > +-host_pair_line totals; > +-int peaksent, peakrecv, peaktotal; > ++static sorted_list_type screen_list; > ++static host_pair_line totals; > ++static int peaksent, peakrecv, peaktotal; > + extern history_type history_totals; > +-hash_type* screen_hash; > +-hash_type* service_hash; > ++static hash_type* screen_hash; > ++static hash_type* service_hash; Declaring these variables as static in a header file seems wrong to me. Shouldn't this be declared as a global variable in one of the .c files and extern here? - Felix
> On Jul 17, 2020, at 3:06 AM, Felix Fietkau <nbd@nbd.name> wrote: > > On 2020-07-14 07:43, Rosen Penev wrote: >> GCC 10 defaults to fno-common, which demains unique defenitions. >> >> Signed-off-by: Rosen Penev <rosenp@gmail.com> >> --- >> package/network/utils/iftop/Makefile | 2 +- >> .../utils/iftop/patches/010-gcc10.patch | 20 +++++++++++++++++++ >> 2 files changed, 21 insertions(+), 1 deletion(-) >> create mode 100644 package/network/utils/iftop/patches/010-gcc10.patch >> >> diff --git a/package/network/utils/iftop/Makefile b/package/network/utils/iftop/Makefile >> index 98fe15c8f5..aa467c2876 100644 >> --- a/package/network/utils/iftop/Makefile >> +++ b/package/network/utils/iftop/Makefile >> @@ -8,7 +8,7 @@ >> include $(TOPDIR)/rules.mk >> >> PKG_NAME:=iftop >> -PKG_RELEASE:=1 >> +PKG_RELEASE:=2 >> >> PKG_SOURCE_PROTO:=git >> PKG_SOURCE_URL:=https://code.blinkace.com/pdw/iftop.git >> diff --git a/package/network/utils/iftop/patches/010-gcc10.patch b/package/network/utils/iftop/patches/010-gcc10.patch >> new file mode 100644 >> index 0000000000..882565a039 >> --- /dev/null >> +++ b/package/network/utils/iftop/patches/010-gcc10.patch >> @@ -0,0 +1,20 @@ >> +--- a/ui_common.h >> ++++ b/ui_common.h >> +@@ -33,12 +33,12 @@ typedef struct host_pair_line_tag { >> + >> + extern options_t options; >> + >> +-sorted_list_type screen_list; >> +-host_pair_line totals; >> +-int peaksent, peakrecv, peaktotal; >> ++static sorted_list_type screen_list; >> ++static host_pair_line totals; >> ++static int peaksent, peakrecv, peaktotal; >> + extern history_type history_totals; >> +-hash_type* screen_hash; >> +-hash_type* service_hash; >> ++static hash_type* screen_hash; >> ++static hash_type* service_hash; > Declaring these variables as static in a header file seems wrong to me. > Shouldn't this be declared as a global variable in one of the .c files > and extern here? Sure. static creates a smaller patch though. > > - Felix
Hi, I've sent a fix using extern declarations upstream, lets see what happens. ~ Jo
> -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] > On Behalf Of Petr Štetiar > Sent: Freitag, 17. Juli 2020 11:50 > To: Rosen Penev <rosenp@gmail.com> > Cc: openwrt-devel@lists.openwrt.org > Subject: Re: [PATCH] iftop: fix compilation with GCC 10 > > Rosen Penev <rosenp@gmail.com> [2020-07-13 22:43:51]: > > Hi, > > > GCC 10 defaults to fno-common, which demains unique defenitions. > > whats the exact error here? Wouldn't it make sense to move this into > packages feed? I couldn't find any reference to/dependency on that package anywhere in main repo, and description there also describes it as something nice-to-have-when-you-need-it. Sounds like packages feed material to me. Best Adrian > What about upstream, looks like they might find this patch handy. > > -- ynezz > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On 17.07.20 11:56, Rosen Penev wrote: > >> On Jul 17, 2020, at 2:50 AM, Petr Štetiar <ynezz@true.cz> wrote: >> >> Rosen Penev <rosenp@gmail.com> [2020-07-13 22:43:51]: >> >> Hi, >> >>> GCC 10 defaults to fno-common, which demains unique defenitions. >> whats the exact error here? Wouldn't it make sense to move this into packages feed? +1 > No idea what this package is for or the reason for its presence here. It's a ncurses based tool which shows how much bandwidth is going to a specific destination (shows the DNS name if there is one) Pretty handy to check in realtime how much traffic is going to specific sites >> What about upstream, looks like they might find this patch handy. > Seems relatively dead upstream. Last commit one year ago and second to last three years ago. >> -- ynezz > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Rosen Penev <rosenp@gmail.com> writes: >> On Jul 17, 2020, at 3:06 AM, Felix Fietkau <nbd@nbd.name> wrote: >>> +--- a/ui_common.h >>> ++++ b/ui_common.h >>> +@@ -33,12 +33,12 @@ typedef struct host_pair_line_tag { >>> + >>> + extern options_t options; >>> + >>> +-sorted_list_type screen_list; >>> +-host_pair_line totals; >>> +-int peaksent, peakrecv, peaktotal; >>> ++static sorted_list_type screen_list; >>> ++static host_pair_line totals; >>> ++static int peaksent, peakrecv, peaktotal; >>> + extern history_type history_totals; >>> +-hash_type* screen_hash; >>> +-hash_type* service_hash; >>> ++static hash_type* screen_hash; >>> ++static hash_type* service_hash; >> >> Declaring these variables as static in a header file seems wrong to me. >> Shouldn't this be declared as a global variable in one of the .c files >> and extern here? > > Sure. static creates a smaller patch though. How does that help if the result is buggy? I assume these variables are declared in a header because their values are actually shared. Bjørn
On Fri, Jul 17, 2020 at 3:21 AM Jo-Philipp Wich <jo@mein.io> wrote: > > Hi, > > I've sent a fix using extern declarations upstream, lets see what happens. Where? I don't see a merge request on the git link referenced in the Makefile. > > ~ Jo > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On Fri, Jul 17, 2020 at 3:49 AM Bjørn Mork <bjorn@mork.no> wrote: > > Rosen Penev <rosenp@gmail.com> writes: > >> On Jul 17, 2020, at 3:06 AM, Felix Fietkau <nbd@nbd.name> wrote: > > >>> +--- a/ui_common.h > >>> ++++ b/ui_common.h > >>> +@@ -33,12 +33,12 @@ typedef struct host_pair_line_tag { > >>> + > >>> + extern options_t options; > >>> + > >>> +-sorted_list_type screen_list; > >>> +-host_pair_line totals; > >>> +-int peaksent, peakrecv, peaktotal; > >>> ++static sorted_list_type screen_list; > >>> ++static host_pair_line totals; > >>> ++static int peaksent, peakrecv, peaktotal; > >>> + extern history_type history_totals; > >>> +-hash_type* screen_hash; > >>> +-hash_type* service_hash; > >>> ++static hash_type* screen_hash; > >>> ++static hash_type* service_hash; > >> > >> Declaring these variables as static in a header file seems wrong to me. > >> Shouldn't this be declared as a global variable in one of the .c files > >> and extern here? > > > > Sure. static creates a smaller patch though. > > How does that help if the result is buggy? I assume these variables are > declared in a header because their values are actually shared. Correct. -fno-common (the default in GCC10) mandates that variable names be unique. No two variables can share the same name. Interestingly enough, clang has a warning for this (-Wmissing-variable-declarations). > > > Bjørn > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Hi,
> Where? I don't see a merge request on the git link referenced in the Makefile.
sent a patch to the author using git send-email. Couldn't figure out how
to open a MR on that Gitlab thing after looking for 30 seconds.
~ Jo
diff --git a/package/network/utils/iftop/Makefile b/package/network/utils/iftop/Makefile index 98fe15c8f5..aa467c2876 100644 --- a/package/network/utils/iftop/Makefile +++ b/package/network/utils/iftop/Makefile @@ -8,7 +8,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=iftop -PKG_RELEASE:=1 +PKG_RELEASE:=2 PKG_SOURCE_PROTO:=git PKG_SOURCE_URL:=https://code.blinkace.com/pdw/iftop.git diff --git a/package/network/utils/iftop/patches/010-gcc10.patch b/package/network/utils/iftop/patches/010-gcc10.patch new file mode 100644 index 0000000000..882565a039 --- /dev/null +++ b/package/network/utils/iftop/patches/010-gcc10.patch @@ -0,0 +1,20 @@ +--- a/ui_common.h ++++ b/ui_common.h +@@ -33,12 +33,12 @@ typedef struct host_pair_line_tag { + + extern options_t options; + +-sorted_list_type screen_list; +-host_pair_line totals; +-int peaksent, peakrecv, peaktotal; ++static sorted_list_type screen_list; ++static host_pair_line totals; ++static int peaksent, peakrecv, peaktotal; + extern history_type history_totals; +-hash_type* screen_hash; +-hash_type* service_hash; ++static hash_type* screen_hash; ++static hash_type* service_hash; + + void analyse_data(void); + void screen_list_init(void);
GCC 10 defaults to fno-common, which demains unique defenitions. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- package/network/utils/iftop/Makefile | 2 +- .../utils/iftop/patches/010-gcc10.patch | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 package/network/utils/iftop/patches/010-gcc10.patch