| Message ID | 20211117121142.316696-1-roid@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] netdev-vport: Fix compilation warning | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On Wed, Nov 17, 2021 at 1:12 PM Roi Dayan via dev <ovs-dev@openvswitch.org> wrote: > > port is declared as uint16 so use %hu specifier instead of %d. > > lib/netdev-vport.c:460:44: error: '%s' directive output may be truncated writing up to 4 bytes into a region of size between 1 and 10 [-Werror=format-truncation=] > snprintf(namebuf, bufsize, "dst_port_%d%s", > ^~ Out of curiosity, is it due to new checks from a recent compiler? > Fixes: 189de33f02b2 ("netdev-vport: reject concomitant incompatible tunnels") > Signed-off-by: Roi Dayan <roid@nvidia.com> > Reviewed-by: Eli Britstein <elibr@nvidia.com> > --- > lib/netdev-vport.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c > index 499c0291c933..1c7f55757e9a 100644 > --- a/lib/netdev-vport.c > +++ b/lib/netdev-vport.c > @@ -457,7 +457,7 @@ static char * > vxlan_get_port_ext_gbp_str(uint16_t port, bool gbp, > char namebuf[], size_t bufsize) > { > - snprintf(namebuf, bufsize, "dst_port_%d%s", > + snprintf(namebuf, bufsize, "dst_port_%hu%s", > port, gbp ? "_gbp" : ""); Format for uint16_t is PRIu16. > > return namebuf;
On 2021-11-17 2:17 PM, David Marchand wrote: > On Wed, Nov 17, 2021 at 1:12 PM Roi Dayan via dev > <ovs-dev@openvswitch.org> wrote: >> >> port is declared as uint16 so use %hu specifier instead of %d. >> >> lib/netdev-vport.c:460:44: error: '%s' directive output may be truncated writing up to 4 bytes into a region of size between 1 and 10 [-Werror=format-truncation=] >> snprintf(namebuf, bufsize, "dst_port_%d%s", >> ^~ > > Out of curiosity, is it due to new checks from a recent compiler? do you mean if we updated the compiler? than no. it fails on powerpc machine. > > >> Fixes: 189de33f02b2 ("netdev-vport: reject concomitant incompatible tunnels") >> Signed-off-by: Roi Dayan <roid@nvidia.com> >> Reviewed-by: Eli Britstein <elibr@nvidia.com> >> --- >> lib/netdev-vport.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >> index 499c0291c933..1c7f55757e9a 100644 >> --- a/lib/netdev-vport.c >> +++ b/lib/netdev-vport.c >> @@ -457,7 +457,7 @@ static char * >> vxlan_get_port_ext_gbp_str(uint16_t port, bool gbp, >> char namebuf[], size_t bufsize) >> { >> - snprintf(namebuf, bufsize, "dst_port_%d%s", >> + snprintf(namebuf, bufsize, "dst_port_%hu%s", >> port, gbp ? "_gbp" : ""); > > Format for uint16_t is PRIu16. tested PRIu16 and got the same error. isn't PRIu16 defined as "u" ? > >> >> return namebuf; > >
On 2021-11-17 2:23 PM, Roi Dayan wrote: > > > On 2021-11-17 2:17 PM, David Marchand wrote: >> On Wed, Nov 17, 2021 at 1:12 PM Roi Dayan via dev >> <ovs-dev@openvswitch.org> wrote: >>> >>> port is declared as uint16 so use %hu specifier instead of %d. >>> >>> lib/netdev-vport.c:460:44: error: '%s' directive output may be >>> truncated writing up to 4 bytes into a region of size between 1 and >>> 10 [-Werror=format-truncation=] >>> snprintf(namebuf, bufsize, "dst_port_%d%s", >>> ^~ >> >> Out of curiosity, is it due to new checks from a recent compiler? > > do you mean if we updated the compiler? than no. > it fails on powerpc machine. > >> >> >>> Fixes: 189de33f02b2 ("netdev-vport: reject concomitant incompatible >>> tunnels") >>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>> Reviewed-by: Eli Britstein <elibr@nvidia.com> >>> --- >>> lib/netdev-vport.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >>> index 499c0291c933..1c7f55757e9a 100644 >>> --- a/lib/netdev-vport.c >>> +++ b/lib/netdev-vport.c >>> @@ -457,7 +457,7 @@ static char * >>> vxlan_get_port_ext_gbp_str(uint16_t port, bool gbp, >>> char namebuf[], size_t bufsize) >>> { >>> - snprintf(namebuf, bufsize, "dst_port_%d%s", >>> + snprintf(namebuf, bufsize, "dst_port_%hu%s", >>> port, gbp ? "_gbp" : ""); >> >> Format for uint16_t is PRIu16. > > tested PRIu16 and got the same error. isn't PRIu16 defined as "u" ? i guess PRIu16 should be platform dependent. for some reason it fails as well. > >> >>> >>> return namebuf; >> >>
On 2021-11-17 2:23 PM, Roi Dayan wrote: > > > On 2021-11-17 2:17 PM, David Marchand wrote: >> On Wed, Nov 17, 2021 at 1:12 PM Roi Dayan via dev >> <ovs-dev@openvswitch.org> wrote: >>> >>> port is declared as uint16 so use %hu specifier instead of %d. >>> >>> lib/netdev-vport.c:460:44: error: '%s' directive output may be >>> truncated writing up to 4 bytes into a region of size between 1 and >>> 10 [-Werror=format-truncation=] >>> snprintf(namebuf, bufsize, "dst_port_%d%s", >>> ^~ >> >> Out of curiosity, is it due to new checks from a recent compiler? > > do you mean if we updated the compiler? than no. > it fails on powerpc machine. > >> >> >>> Fixes: 189de33f02b2 ("netdev-vport: reject concomitant incompatible >>> tunnels") >>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>> Reviewed-by: Eli Britstein <elibr@nvidia.com> >>> --- >>> lib/netdev-vport.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >>> index 499c0291c933..1c7f55757e9a 100644 >>> --- a/lib/netdev-vport.c >>> +++ b/lib/netdev-vport.c >>> @@ -457,7 +457,7 @@ static char * >>> vxlan_get_port_ext_gbp_str(uint16_t port, bool gbp, >>> char namebuf[], size_t bufsize) >>> { >>> - snprintf(namebuf, bufsize, "dst_port_%d%s", >>> + snprintf(namebuf, bufsize, "dst_port_%hu%s", >>> port, gbp ? "_gbp" : ""); >> >> Format for uint16_t is PRIu16. > > tested PRIu16 and got the same error. isn't PRIu16 defined as "u" ? > did some more tests. the test machine is ppc64be. lib/netdev-vport.c:460:51: note: format string is defined here snprintf(namebuf, bufsize, "dst_port_%"PRIu16"%s", ^~ seems the compiler now complains on the string. if i remove the string the compiler doesn't complain with either %d or PRIu16. >> >>> >>> return namebuf; >> >>
On 2021-11-17 2:42 PM, Roi Dayan wrote: > > > On 2021-11-17 2:23 PM, Roi Dayan wrote: >> >> >> On 2021-11-17 2:17 PM, David Marchand wrote: >>> On Wed, Nov 17, 2021 at 1:12 PM Roi Dayan via dev >>> <ovs-dev@openvswitch.org> wrote: >>>> >>>> port is declared as uint16 so use %hu specifier instead of %d. >>>> >>>> lib/netdev-vport.c:460:44: error: '%s' directive output may be >>>> truncated writing up to 4 bytes into a region of size between 1 and >>>> 10 [-Werror=format-truncation=] >>>> snprintf(namebuf, bufsize, "dst_port_%d%s", >>>> ^~ >>> >>> Out of curiosity, is it due to new checks from a recent compiler? >> >> do you mean if we updated the compiler? than no. >> it fails on powerpc machine. >> >>> >>> >>>> Fixes: 189de33f02b2 ("netdev-vport: reject concomitant incompatible >>>> tunnels") >>>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>>> Reviewed-by: Eli Britstein <elibr@nvidia.com> >>>> --- >>>> lib/netdev-vport.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >>>> index 499c0291c933..1c7f55757e9a 100644 >>>> --- a/lib/netdev-vport.c >>>> +++ b/lib/netdev-vport.c >>>> @@ -457,7 +457,7 @@ static char * >>>> vxlan_get_port_ext_gbp_str(uint16_t port, bool gbp, >>>> char namebuf[], size_t bufsize) >>>> { >>>> - snprintf(namebuf, bufsize, "dst_port_%d%s", >>>> + snprintf(namebuf, bufsize, "dst_port_%hu%s", >>>> port, gbp ? "_gbp" : ""); >>> >>> Format for uint16_t is PRIu16. >> >> tested PRIu16 and got the same error. isn't PRIu16 defined as "u" ? >> > > did some more tests. > the test machine is ppc64be. > > lib/netdev-vport.c:460:51: note: format string is defined here > snprintf(namebuf, bufsize, "dst_port_%"PRIu16"%s", > ^~ > > seems the compiler now complains on the string. > if i remove the string the compiler doesn't complain with either %d > or PRIu16. seems i got it wrong. the compiler identify the size passed is 20. %hu solved it by limiting the number and have room for the string "_gbp". using PRIu16 is bigger than %hu allowing to pass the 20 chars size. i'll send a diff fix increasing the buf size to 24 and using PRIu16 as should. thanks for the review. > >>> >>>> >>>> return namebuf; >>> >>>
On 2021-11-17 2:51 PM, Roi Dayan wrote: > > > On 2021-11-17 2:42 PM, Roi Dayan wrote: >> >> >> On 2021-11-17 2:23 PM, Roi Dayan wrote: >>> >>> >>> On 2021-11-17 2:17 PM, David Marchand wrote: >>>> On Wed, Nov 17, 2021 at 1:12 PM Roi Dayan via dev >>>> <ovs-dev@openvswitch.org> wrote: >>>>> >>>>> port is declared as uint16 so use %hu specifier instead of %d. >>>>> >>>>> lib/netdev-vport.c:460:44: error: '%s' directive output may be >>>>> truncated writing up to 4 bytes into a region of size between 1 and >>>>> 10 [-Werror=format-truncation=] >>>>> snprintf(namebuf, bufsize, "dst_port_%d%s", >>>>> ^~ >>>> >>>> Out of curiosity, is it due to new checks from a recent compiler? >>> >>> do you mean if we updated the compiler? than no. >>> it fails on powerpc machine. >>> >>>> >>>> >>>>> Fixes: 189de33f02b2 ("netdev-vport: reject concomitant incompatible >>>>> tunnels") >>>>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>>>> Reviewed-by: Eli Britstein <elibr@nvidia.com> >>>>> --- >>>>> lib/netdev-vport.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >>>>> index 499c0291c933..1c7f55757e9a 100644 >>>>> --- a/lib/netdev-vport.c >>>>> +++ b/lib/netdev-vport.c >>>>> @@ -457,7 +457,7 @@ static char * >>>>> vxlan_get_port_ext_gbp_str(uint16_t port, bool gbp, >>>>> char namebuf[], size_t bufsize) >>>>> { >>>>> - snprintf(namebuf, bufsize, "dst_port_%d%s", >>>>> + snprintf(namebuf, bufsize, "dst_port_%hu%s", >>>>> port, gbp ? "_gbp" : ""); >>>> >>>> Format for uint16_t is PRIu16. >>> >>> tested PRIu16 and got the same error. isn't PRIu16 defined as "u" ? >>> >> >> did some more tests. >> the test machine is ppc64be. >> >> lib/netdev-vport.c:460:51: note: format string is defined here >> snprintf(namebuf, bufsize, "dst_port_%"PRIu16"%s", >> ^~ >> >> seems the compiler now complains on the string. >> if i remove the string the compiler doesn't complain with either %d >> or PRIu16. > > seems i got it wrong. the compiler identify the size passed is 20. > %hu solved it by limiting the number and have room for the string "_gbp". > using PRIu16 is bigger than %hu allowing to pass the 20 chars size. > i'll send a diff fix increasing the buf size to 24 and using PRIu16 > as should. > > thanks for the review. > > before submitting i'm trying to understand why the compiler suggests 24. "dst_port_" is 9 and the port could be 2. then "_gbp" is 4. so 20 should be enough. someone has an idea? I can reproduce the issue only on ppc64be. >> >>>> >>>>> >>>>> return namebuf; >>>> >>>>
On 17 Nov 2021, at 14:00, Roi Dayan via dev wrote: > On 2021-11-17 2:51 PM, Roi Dayan wrote: >> >> >> On 2021-11-17 2:42 PM, Roi Dayan wrote: >>> >>> >>> On 2021-11-17 2:23 PM, Roi Dayan wrote: >>>> >>>> >>>> On 2021-11-17 2:17 PM, David Marchand wrote: >>>>> On Wed, Nov 17, 2021 at 1:12 PM Roi Dayan via dev >>>>> <ovs-dev@openvswitch.org> wrote: >>>>>> >>>>>> port is declared as uint16 so use %hu specifier instead of %d. >>>>>> >>>>>> lib/netdev-vport.c:460:44: error: '%s' directive output may be truncated writing up to 4 bytes into a region of size between 1 and 10 [-Werror=format-truncation=] >>>>>> snprintf(namebuf, bufsize, "dst_port_%d%s", >>>>>> ^~ >>>>> >>>>> Out of curiosity, is it due to new checks from a recent compiler? >>>> >>>> do you mean if we updated the compiler? than no. >>>> it fails on powerpc machine. >>>> >>>>> >>>>> >>>>>> Fixes: 189de33f02b2 ("netdev-vport: reject concomitant incompatible tunnels") >>>>>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>>>>> Reviewed-by: Eli Britstein <elibr@nvidia.com> >>>>>> --- >>>>>> lib/netdev-vport.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >>>>>> index 499c0291c933..1c7f55757e9a 100644 >>>>>> --- a/lib/netdev-vport.c >>>>>> +++ b/lib/netdev-vport.c >>>>>> @@ -457,7 +457,7 @@ static char * >>>>>> vxlan_get_port_ext_gbp_str(uint16_t port, bool gbp, >>>>>> char namebuf[], size_t bufsize) >>>>>> { >>>>>> - snprintf(namebuf, bufsize, "dst_port_%d%s", >>>>>> + snprintf(namebuf, bufsize, "dst_port_%hu%s", >>>>>> port, gbp ? "_gbp" : ""); >>>>> >>>>> Format for uint16_t is PRIu16. >>>> >>>> tested PRIu16 and got the same error. isn't PRIu16 defined as "u" ? >>>> >>> >>> did some more tests. >>> the test machine is ppc64be. >>> >>> lib/netdev-vport.c:460:51: note: format string is defined here >>> snprintf(namebuf, bufsize, "dst_port_%"PRIu16"%s", >>> ^~ >>> >>> seems the compiler now complains on the string. >>> if i remove the string the compiler doesn't complain with either %d >>> or PRIu16. >> >> seems i got it wrong. the compiler identify the size passed is 20. >> %hu solved it by limiting the number and have room for the string "_gbp". >> using PRIu16 is bigger than %hu allowing to pass the 20 chars size. >> i'll send a diff fix increasing the buf size to 24 and using PRIu16 >> as should. >> >> thanks for the review. >> >> > > before submitting i'm trying to understand why the compiler suggests 24. > "dst_port_" is 9 and the port could be 2. then "_gbp" is 4. so 20 > should be enough. someone has an idea? > I can reproduce the issue only on ppc64be. Don’t have a ppc64be compiler ready, but what is the size of an int on that architecture? %d normally is an int, so if 32 bit the max value for the string could be strlen(“-2147483648”). //Eelco
On 17/11/2021 14:44, Eelco Chaudron wrote: > > > On 17 Nov 2021, at 14:00, Roi Dayan via dev wrote: > >> On 2021-11-17 2:51 PM, Roi Dayan wrote: >>> >>> >>> On 2021-11-17 2:42 PM, Roi Dayan wrote: >>>> >>>> >>>> On 2021-11-17 2:23 PM, Roi Dayan wrote: >>>>> >>>>> >>>>> On 2021-11-17 2:17 PM, David Marchand wrote: >>>>>> On Wed, Nov 17, 2021 at 1:12 PM Roi Dayan via dev >>>>>> <ovs-dev@openvswitch.org> wrote: >>>>>>> >>>>>>> port is declared as uint16 so use %hu specifier instead of %d. >>>>>>> >>>>>>> lib/netdev-vport.c:460:44: error: '%s' directive output may be truncated writing up to 4 bytes into a region of size between 1 and 10 [-Werror=format-truncation=] >>>>>>> snprintf(namebuf, bufsize, "dst_port_%d%s", >>>>>>> ^~ >>>>>> >>>>>> Out of curiosity, is it due to new checks from a recent compiler? >>>>> >>>>> do you mean if we updated the compiler? than no. >>>>> it fails on powerpc machine. >>>>> >>>>>> >>>>>> >>>>>>> Fixes: 189de33f02b2 ("netdev-vport: reject concomitant incompatible tunnels") >>>>>>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>>>>>> Reviewed-by: Eli Britstein <elibr@nvidia.com> >>>>>>> --- >>>>>>> lib/netdev-vport.c | 2 +- >>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >>>>>>> index 499c0291c933..1c7f55757e9a 100644 >>>>>>> --- a/lib/netdev-vport.c >>>>>>> +++ b/lib/netdev-vport.c >>>>>>> @@ -457,7 +457,7 @@ static char * >>>>>>> vxlan_get_port_ext_gbp_str(uint16_t port, bool gbp, >>>>>>> char namebuf[], size_t bufsize) >>>>>>> { >>>>>>> - snprintf(namebuf, bufsize, "dst_port_%d%s", >>>>>>> + snprintf(namebuf, bufsize, "dst_port_%hu%s", >>>>>>> port, gbp ? "_gbp" : ""); >>>>>> >>>>>> Format for uint16_t is PRIu16. >>>>> >>>>> tested PRIu16 and got the same error. isn't PRIu16 defined as "u" ? >>>>> >>>> >>>> did some more tests. >>>> the test machine is ppc64be. >>>> >>>> lib/netdev-vport.c:460:51: note: format string is defined here >>>> snprintf(namebuf, bufsize, "dst_port_%"PRIu16"%s", >>>> ^~ >>>> >>>> seems the compiler now complains on the string. >>>> if i remove the string the compiler doesn't complain with either %d >>>> or PRIu16. >>> >>> seems i got it wrong. the compiler identify the size passed is 20. >>> %hu solved it by limiting the number and have room for the string "_gbp". >>> using PRIu16 is bigger than %hu allowing to pass the 20 chars size. >>> i'll send a diff fix increasing the buf size to 24 and using PRIu16 >>> as should. >>> >>> thanks for the review. >>> >>> >> >> before submitting i'm trying to understand why the compiler suggests 24. >> "dst_port_" is 9 and the port could be 2. then "_gbp" is 4. so 20 >> should be enough. someone has an idea? >> I can reproduce the issue only on ppc64be. > > Don’t have a ppc64be compiler ready, but what is the size of an int on that architecture? > %d normally is an int, so if 32 bit the max value for the string could be strlen(“-2147483648”). > There is an INT_STRLEN() macro in https://github.com/openvswitch/ovs/blob/master/include/openvswitch/type-props.h which is used elsewhere in the code and might be useful here. > //Eelco > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 2021-11-17 5:45 PM, Kevin Traynor wrote: > On 17/11/2021 14:44, Eelco Chaudron wrote: >> >> >> On 17 Nov 2021, at 14:00, Roi Dayan via dev wrote: >> >>> On 2021-11-17 2:51 PM, Roi Dayan wrote: >>>> >>>> >>>> On 2021-11-17 2:42 PM, Roi Dayan wrote: >>>>> >>>>> >>>>> On 2021-11-17 2:23 PM, Roi Dayan wrote: >>>>>> >>>>>> >>>>>> On 2021-11-17 2:17 PM, David Marchand wrote: >>>>>>> On Wed, Nov 17, 2021 at 1:12 PM Roi Dayan via dev >>>>>>> <ovs-dev@openvswitch.org> wrote: >>>>>>>> >>>>>>>> port is declared as uint16 so use %hu specifier instead of %d. >>>>>>>> >>>>>>>> lib/netdev-vport.c:460:44: error: '%s' directive output may be >>>>>>>> truncated writing up to 4 bytes into a region of size between 1 >>>>>>>> and 10 [-Werror=format-truncation=] >>>>>>>> snprintf(namebuf, bufsize, "dst_port_%d%s", >>>>>>>> ^~ >>>>>>> >>>>>>> Out of curiosity, is it due to new checks from a recent compiler? >>>>>> >>>>>> do you mean if we updated the compiler? than no. >>>>>> it fails on powerpc machine. >>>>>> >>>>>>> >>>>>>> >>>>>>>> Fixes: 189de33f02b2 ("netdev-vport: reject concomitant >>>>>>>> incompatible tunnels") >>>>>>>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>>>>>>> Reviewed-by: Eli Britstein <elibr@nvidia.com> >>>>>>>> --- >>>>>>>> lib/netdev-vport.c | 2 +- >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >>>>>>>> index 499c0291c933..1c7f55757e9a 100644 >>>>>>>> --- a/lib/netdev-vport.c >>>>>>>> +++ b/lib/netdev-vport.c >>>>>>>> @@ -457,7 +457,7 @@ static char * >>>>>>>> vxlan_get_port_ext_gbp_str(uint16_t port, bool gbp, >>>>>>>> char namebuf[], size_t bufsize) >>>>>>>> { >>>>>>>> - snprintf(namebuf, bufsize, "dst_port_%d%s", >>>>>>>> + snprintf(namebuf, bufsize, "dst_port_%hu%s", >>>>>>>> port, gbp ? "_gbp" : ""); >>>>>>> >>>>>>> Format for uint16_t is PRIu16. >>>>>> >>>>>> tested PRIu16 and got the same error. isn't PRIu16 defined as "u" ? >>>>>> >>>>> >>>>> did some more tests. >>>>> the test machine is ppc64be. >>>>> >>>>> lib/netdev-vport.c:460:51: note: format string is defined here >>>>> snprintf(namebuf, bufsize, "dst_port_%"PRIu16"%s", >>>>> ^~ >>>>> >>>>> seems the compiler now complains on the string. >>>>> if i remove the string the compiler doesn't complain with either %d >>>>> or PRIu16. >>>> >>>> seems i got it wrong. the compiler identify the size passed is 20. >>>> %hu solved it by limiting the number and have room for the string >>>> "_gbp". >>>> using PRIu16 is bigger than %hu allowing to pass the 20 chars size. >>>> i'll send a diff fix increasing the buf size to 24 and using PRIu16 >>>> as should. >>>> >>>> thanks for the review. >>>> >>>> >>> >>> before submitting i'm trying to understand why the compiler suggests 24. >>> "dst_port_" is 9 and the port could be 2. then "_gbp" is 4. so 20 >>> should be enough. someone has an idea? >>> I can reproduce the issue only on ppc64be. >> >> Don’t have a ppc64be compiler ready, but what is the size of an int on >> that architecture? >> %d normally is an int, so if 32 bit the max value for the string could >> be strlen(“-2147483648”). >> > > There is an INT_STRLEN() macro in > https://github.com/openvswitch/ovs/blob/master/include/openvswitch/type-props.h which is used elsewhere in the code and might be useful here. > thanks. I checked size for int, uint16 and char with this macro. 2021-11-18T07:57:28Z|00020|dpif_netlink|INFO|XXX size uint16_t 6 2021-11-18T07:57:28Z|00021|dpif_netlink|INFO|XXX size int 12 2021-11-18T07:58:58Z|00047|dpif_netlink|INFO|XXX size char 3 >> //Eelco >> >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >
On 2021-11-18 9:59 AM, Roi Dayan wrote: > > > On 2021-11-17 5:45 PM, Kevin Traynor wrote: >> On 17/11/2021 14:44, Eelco Chaudron wrote: >>> >>> >>> On 17 Nov 2021, at 14:00, Roi Dayan via dev wrote: >>> >>>> On 2021-11-17 2:51 PM, Roi Dayan wrote: >>>>> >>>>> >>>>> On 2021-11-17 2:42 PM, Roi Dayan wrote: >>>>>> >>>>>> >>>>>> On 2021-11-17 2:23 PM, Roi Dayan wrote: >>>>>>> >>>>>>> >>>>>>> On 2021-11-17 2:17 PM, David Marchand wrote: >>>>>>>> On Wed, Nov 17, 2021 at 1:12 PM Roi Dayan via dev >>>>>>>> <ovs-dev@openvswitch.org> wrote: >>>>>>>>> >>>>>>>>> port is declared as uint16 so use %hu specifier instead of %d. >>>>>>>>> >>>>>>>>> lib/netdev-vport.c:460:44: error: '%s' directive output may be >>>>>>>>> truncated writing up to 4 bytes into a region of size between 1 >>>>>>>>> and 10 [-Werror=format-truncation=] >>>>>>>>> snprintf(namebuf, bufsize, "dst_port_%d%s", >>>>>>>>> ^~ >>>>>>>> >>>>>>>> Out of curiosity, is it due to new checks from a recent compiler? >>>>>>> >>>>>>> do you mean if we updated the compiler? than no. >>>>>>> it fails on powerpc machine. >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> Fixes: 189de33f02b2 ("netdev-vport: reject concomitant >>>>>>>>> incompatible tunnels") >>>>>>>>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>>>>>>>> Reviewed-by: Eli Britstein <elibr@nvidia.com> >>>>>>>>> --- >>>>>>>>> lib/netdev-vport.c | 2 +- >>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>>> >>>>>>>>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >>>>>>>>> index 499c0291c933..1c7f55757e9a 100644 >>>>>>>>> --- a/lib/netdev-vport.c >>>>>>>>> +++ b/lib/netdev-vport.c >>>>>>>>> @@ -457,7 +457,7 @@ static char * >>>>>>>>> vxlan_get_port_ext_gbp_str(uint16_t port, bool gbp, >>>>>>>>> char namebuf[], size_t bufsize) >>>>>>>>> { >>>>>>>>> - snprintf(namebuf, bufsize, "dst_port_%d%s", >>>>>>>>> + snprintf(namebuf, bufsize, "dst_port_%hu%s", >>>>>>>>> port, gbp ? "_gbp" : ""); >>>>>>>> >>>>>>>> Format for uint16_t is PRIu16. >>>>>>> >>>>>>> tested PRIu16 and got the same error. isn't PRIu16 defined as "u" ? >>>>>>> >>>>>> >>>>>> did some more tests. >>>>>> the test machine is ppc64be. >>>>>> >>>>>> lib/netdev-vport.c:460:51: note: format string is defined here >>>>>> snprintf(namebuf, bufsize, "dst_port_%"PRIu16"%s", >>>>>> ^~ >>>>>> >>>>>> seems the compiler now complains on the string. >>>>>> if i remove the string the compiler doesn't complain with either %d >>>>>> or PRIu16. >>>>> >>>>> seems i got it wrong. the compiler identify the size passed is 20. >>>>> %hu solved it by limiting the number and have room for the string >>>>> "_gbp". >>>>> using PRIu16 is bigger than %hu allowing to pass the 20 chars size. >>>>> i'll send a diff fix increasing the buf size to 24 and using PRIu16 >>>>> as should. >>>>> >>>>> thanks for the review. >>>>> >>>>> >>>> >>>> before submitting i'm trying to understand why the compiler suggests >>>> 24. >>>> "dst_port_" is 9 and the port could be 2. then "_gbp" is 4. so 20 >>>> should be enough. someone has an idea? >>>> I can reproduce the issue only on ppc64be. >>> >>> Don’t have a ppc64be compiler ready, but what is the size of an int >>> on that architecture? >>> %d normally is an int, so if 32 bit the max value for the string >>> could be strlen(“-2147483648”). >>> >> >> There is an INT_STRLEN() macro in >> https://github.com/openvswitch/ovs/blob/master/include/openvswitch/type-props.h which is used elsewhere in the code and might be useful here. >> > > thanks. I checked size for int, uint16 and char with this macro. > > 2021-11-18T07:57:28Z|00020|dpif_netlink|INFO|XXX size uint16_t 6 > 2021-11-18T07:57:28Z|00021|dpif_netlink|INFO|XXX size int 12 > 2021-11-18T07:58:58Z|00047|dpif_netlink|INFO|XXX size char 3 > strlen seems to give expected result dpif_netlink|INFO|XXX size strlen("-2147483648") 11 > >>> //Eelco >>> >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >>
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 499c0291c933..1c7f55757e9a 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -457,7 +457,7 @@ static char * vxlan_get_port_ext_gbp_str(uint16_t port, bool gbp, char namebuf[], size_t bufsize) { - snprintf(namebuf, bufsize, "dst_port_%d%s", + snprintf(namebuf, bufsize, "dst_port_%hu%s", port, gbp ? "_gbp" : ""); return namebuf;
port is declared as uint16 so use %hu specifier instead of %d. lib/netdev-vport.c:460:44: error: '%s' directive output may be truncated writing up to 4 bytes into a region of size between 1 and 10 [-Werror=format-truncation=] snprintf(namebuf, bufsize, "dst_port_%d%s", ^~ Fixes: 189de33f02b2 ("netdev-vport: reject concomitant incompatible tunnels") Signed-off-by: Roi Dayan <roid@nvidia.com> Reviewed-by: Eli Britstein <elibr@nvidia.com> --- lib/netdev-vport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)