keith has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/21118 )
Change subject: Configure E1 pcap file per line ...................................................................... Patch Set 8: (7 comments) Thanks. https://gerrit.osmocom.org/c/libosmo-abis/+/21118/7/include/osmocom/abis/e1_input.h File include/osmocom/abis/e1_input.h: https://gerrit.osmocom.org/c/libosmo-abis/+/21118/7/include/osmocom/abis/e1_input.h@324 PS7, Line 324: e1_set_pcap_fd > Should we deprecate the old symbol now? What does it imply to deprecate the old symbol here? To add a deprecated compliler warning? BTW, AFAIK, the only place this is called is in openbsc - osmo-nitb/bsc_hack.c https://gerrit.osmocom.org/c/libosmo-abis/+/21118/3/include/osmocom/abis/e1_input.h File include/osmocom/abis/e1_input.h: https://gerrit.osmocom.org/c/libosmo-abis/+/21118/3/include/osmocom/abis/e1_input.h@319 PS3, Line 319: int e1_set_pcap_fd(int fd, int e1_nr); > Yes a new function would be great to avoid breaking build in osmo-nitb. […] Done https://gerrit.osmocom.org/c/libosmo-abis/+/21118/3/src/e1_input.c File src/e1_input.c: https://gerrit.osmocom.org/c/libosmo-abis/+/21118/3/src/e1_input.c@186 PS3, Line 186: struct msgb *msg, int pcap_fd) { > Ah sorry I thought it's not because I didn't see any line added with it. Ack https://gerrit.osmocom.org/c/libosmo-abis/+/21118/7/src/e1_input.c File src/e1_input.c: https://gerrit.osmocom.org/c/libosmo-abis/+/21118/7/src/e1_input.c@144 PS7, Line 144: pcap_hdr > It can also be 'static' I think, so it will definitely end up in RODATA. Done https://gerrit.osmocom.org/c/libosmo-abis/+/21118/7/src/e1_input_vty.c File src/e1_input_vty.c: https://gerrit.osmocom.org/c/libosmo-abis/+/21118/7/src/e1_input_vty.c@273 PS7, Line 273: return CMD_WARNING; > I think you need to close(fd) before you return from here. Done https://gerrit.osmocom.org/c/libosmo-abis/+/21118/7/src/e1_input_vty.c@275 PS7, Line 275: if (line->pcap_file) { > osmo_talloc_replace_string() will free() it automatically, remove this block. Done https://gerrit.osmocom.org/c/libosmo-abis/+/21118/7/src/e1_input_vty.c@295 PS7, Line 295: void > This pointer cast is not needed because it's not 'const'. Done -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/21118 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I316c3d6a839e84c2f52a148c6b8dd6f5933cf4bf Gerrit-Change-Number: 21118 Gerrit-PatchSet: 8 Gerrit-Owner: keith <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: keith <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-CC: laforge <[email protected]> Gerrit-Comment-Date: Wed, 14 Apr 2021 19:41:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <[email protected]> Comment-In-Reply-To: fixeria <[email protected]> Comment-In-Reply-To: keith <[email protected]> Gerrit-MessageType: comment
