tnt has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmo-abis/+/18114 )

Change subject: e1_input: Add VTY command to enable PCAP debug output
......................................................................


Patch Set 3:

(3 comments)

https://gerrit.osmocom.org/c/libosmo-abis/+/18114/1/src/e1_input_vty.c
File src/e1_input_vty.c:

https://gerrit.osmocom.org/c/libosmo-abis/+/18114/1/src/e1_input_vty.c@249
PS1, Line 249:          vty_out(vty, "Failed to setup E1 pcap recording to 
%s.%s", argv[1], VTY_NEWLINE);
> argv[0]
Oops. Will fix that.


https://gerrit.osmocom.org/c/libosmo-abis/+/18114/1/src/e1_input_vty.c@254
PS1, Line 254:  pcap_filename = talloc_strdup(NULL, argv[0]);
> We try not to allocated on NULL ctx. […]
It's in e1_input.c but limited to that file, I'd need to bring it to the global 
namespace just for that :/

At that point I'd rather just not keep the filename at all and the 'write' 
config would just no include the filename. This is just a debug feature anyway.


https://gerrit.osmocom.org/c/libosmo-abis/+/18114/1/src/e1_input_vty.c@267
PS1, Line 267:  e1_set_pcap_fd(-1);
> Don't you need to close the previous fd?
We can't.

It's explained in the commit logs, but this only affects the fd of _future_ 
lines, the active ones will continue to use whatever was set previously, so you 
can't close the fd since it's still being used.



--
To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/18114
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I0b4251702aecd6721b9d63c320351ef6cb513454
Gerrit-Change-Number: 18114
Gerrit-PatchSet: 3
Gerrit-Owner: tnt <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-Reviewer: tnt <[email protected]>
Gerrit-Comment-Date: Fri, 08 May 2020 12:57:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <[email protected]>
Gerrit-MessageType: comment

Reply via email to