pimd: bump pim cli XPath buffers size #10367
Conversation
Bump the size of the buffers so the new compilers don't complain about possible truncation: Signed-off-by: sarita patra <saritap@vmware.com>
|
@donaldsharp, I am not able to see the truncation issue in the version which I am currently using So I have not verified this fix. |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: IncompleteDebian 10 amd64 build: Incomplete(check logs for details)Successful on other platforms/tests
|
2 similar comments
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: IncompleteDebian 10 amd64 build: Incomplete(check logs for details)Successful on other platforms/tests
|
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: IncompleteDebian 10 amd64 build: Incomplete(check logs for details)Successful on other platforms/tests
|
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: IncompleteAddresssanitizer topotests part 5: Incomplete(check logs for details)Topotests Ubuntu 18.04 amd64 part 3: Incomplete(check logs for details)Topotests Ubuntu 18.04 amd64 part 2: Incomplete(check logs for details)Topotests Ubuntu 18.04 amd64 part 5: Incomplete(check logs for details)IPv4 ldp protocol on Ubuntu 18.04: Incomplete(check logs for details)IPv4 protocols on Ubuntu 18.04: Incomplete(check logs for details)Addresssanitizer topotests part 9: Incomplete(check logs for details)Addresssanitizer topotests part 6: Incomplete(check logs for details)Topotests debian 10 amd64 part 2: Incomplete(check logs for details)Topotests Ubuntu 18.04 amd64 part 8: Incomplete(check logs for details)Topotests Ubuntu 18.04 amd64 part 0: Incomplete(check logs for details)Topotests debian 10 amd64 part 5: Incomplete(check logs for details)Addresssanitizer topotests part 1: Incomplete(check logs for details)Topotests Ubuntu 18.04 i386 part 4: Incomplete(check logs for details)Topotests debian 10 amd64 part 6: Incomplete(check logs for details)Addresssanitizer topotests part 8: Incomplete(check logs for details)Topotests Ubuntu 18.04 amd64 part 6: Incomplete(check logs for details)Successful on other platforms/tests
|
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: IncompleteTopotests Ubuntu 18.04 amd64 part 5: Incomplete(check logs for details)IPv4 ldp protocol on Ubuntu 18.04: Incomplete(check logs for details)Addresssanitizer topotests part 9: Incomplete(check logs for details)Addresssanitizer topotests part 6: Incomplete(check logs for details)Topotests Ubuntu 18.04 amd64 part 8: Incomplete(check logs for details)Topotests debian 10 amd64 part 5: Incomplete(check logs for details)Topotests Ubuntu 18.04 i386 part 4: Incomplete(check logs for details)Successful on other platforms/tests
|
| @@ -7859,7 +7859,7 @@ DEFUN (interface_no_ip_igmp, | |||
| IFACE_IGMP_STR) | |||
| { | |||
| const struct lyd_node *pim_enable_dnode; | |||
| char pim_if_xpath[XPATH_MAXLEN + 20]; | |||
| char pim_if_xpath[XPATH_MAXLEN + 128]; | |||
There was a problem hiding this comment.
where is this number coming from? Adding in a magic number here defeats the purpose of having the macro
There was a problem hiding this comment.
It's coming from the length of the fixed text. XPATH_MAXLEN is how long VTY_CURR_XPATH can be, and we're adding some fixed text to that, which is less than 128 characters long.
There was a problem hiding this comment.
why are we adding 128 @eqvinox ?
There was a problem hiding this comment.
to give space for the pim attribute itself ?
There was a problem hiding this comment.
if it's a fixed value seems like that should be derived from somewhere rather than just pulling it out air to avoid having to change this code again in the future if the data changes, but I will lift my nack nonetheless
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: IncompleteIPv4 ldp protocol on Ubuntu 18.04: Incomplete(check logs for details)Successful on other platforms/tests
|
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-2797/ This is a comment from an automated CI system. |
| @@ -7859,7 +7859,7 @@ DEFUN (interface_no_ip_igmp, | |||
| IFACE_IGMP_STR) | |||
| { | |||
| const struct lyd_node *pim_enable_dnode; | |||
| char pim_if_xpath[XPATH_MAXLEN + 20]; | |||
| char pim_if_xpath[XPATH_MAXLEN + 128]; | |||
There was a problem hiding this comment.
if it's a fixed value seems like that should be derived from somewhere rather than just pulling it out air to avoid having to change this code again in the future if the data changes, but I will lift my nack nonetheless
There was a problem hiding this comment.
I have nacked this kind of change before and I'm nacking it again. As I've explained in the past, taking a constant that denotes the maximum length of something, and then adding to it in order to "make space" for that same thing that supposedly has the max length denoted by the constant, makes no sense.
|
Fixed as part of #10395 |
Bump the size of the buffers so the new compilers don't complain about
possible truncation:
Trying to fix the below truncation issues.
pimd/pim_cmd.c: In function ‘interface_no_ip_igmp’:
pimd/pim_cmd.c:7865:7: error: ‘/frr-pim:pim/address-family[...’ directive output may be truncated writing 44 bytes into a region of size between 21 and 1044 [-Werror=format-truncation=]
7865 | "%s/frr-pim:pim/address-family[address-family='%s']",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pimd/pim_cmd.c:7864:2: note: ‘snprintf’ output between 63 and 1086 bytes into a destination of size 1044
7864 | snprintf(pim_if_xpath, sizeof(pim_if_xpath),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
7865 | "%s/frr-pim:pim/address-family[address-family='%s']",
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
7866 | VTY_CURR_XPATH, "frr-routing:ipv4");
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pimd/pim_cmd.c: In function ‘interface_no_ip_pim_ssm’:
pimd/pim_cmd.c:8412:7: error: ‘/frr-gmp:gmp/address-family[...’ directive output may be truncated writing 44 bytes into a region of size between 21 and 1044 [-Werror=format-truncation=]
8412 | "%s/frr-gmp:gmp/address-family[address-family='%s']",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pimd/pim_cmd.c:8411:2: note: ‘snprintf’ output between 63 and 1086 bytes into a destination of size 1044
8411 | snprintf(igmp_if_xpath, sizeof(igmp_if_xpath),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
8412 | "%s/frr-gmp:gmp/address-family[address-family='%s']",
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
8413 | VTY_CURR_XPATH, "frr-routing:ipv4");
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pimd/pim_cmd.c: In function ‘interface_no_ip_pim_sm’:
pimd/pim_cmd.c:8447:7: error: ‘/frr-gmp:gmp/address-family[...’ directive output may be truncated writing 44 bytes into a region of size between 21 and 1044 [-Werror=format-truncation=]
8447 | "%s/frr-gmp:gmp/address-family[address-family='%s']",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pimd/pim_cmd.c:8446:2: note: ‘snprintf’ output between 63 and 1086 bytes into a destination of size 1044
8446 | snprintf(igmp_if_xpath, sizeof(igmp_if_xpath),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
8447 | "%s/frr-gmp:gmp/address-family[address-family='%s']",
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
8448 | VTY_CURR_XPATH, "frr-routing:ipv4");
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pimd/pim_cmd.c: In function ‘interface_no_ip_pim’:
pimd/pim_cmd.c:8483:7: error: ‘/frr-gmp:gmp/address-family[...’ directive output may be truncated writing 44 bytes into a region of size between 21 and 1044 [-Werror=format-truncation=]
8483 | "%s/frr-gmp:gmp/address-family[address-family='%s']",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pimd/pim_cmd.c:8482:2: note: ‘snprintf’ output between 63 and 1086 bytes into a destination of size 1044
8482 | snprintf(igmp_if_xpath, sizeof(igmp_if_xpath),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
8483 | "%s/frr-gmp:gmp/address-family[address-family='%s']",
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
8484 | VTY_CURR_XPATH, "frr-routing:ipv4");
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Signed-off-by: sarita patra saritap@vmware.com