Patch Set 1: (1 comment)
Hi sorry, it seems I wrote a comment a few days ago but I didn't "submit it". https://gerrit.osmocom.org/#/c/3537/1/src/osmux.c File src/osmux.c: PS1, Line 849: > I'm still trying to understand this update, not sure what corner case you'r I first submited this patch which was already merged, because I was getting a crash: https://gerrit.osmocom.org/#/c/3521/1 But now after seeing the examples in nftables I realize that actually the problem may be that osmux_snprintf_header() and osmux_snprintf_payload() are called passing parameter "size" to them, but they should be called with parameter "len". To be honest, I find these len and size variables quite confusing and here's proof of it. There's afaik no need to maintain 4 state variables instead of 3, so I'd prefer having the new implementation as I think it's easier to follow. I'm OK too with sending a new patchset reverting #3521 and a new patch to s/size/len/ when calling the functions state above, and another patch adding documentation and cosmetics. -- To view, visit https://gerrit.osmocom.org/3537 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I695771d099833842db37a415b636035d17f1bba7 Gerrit-PatchSet: 1 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <[email protected]> Gerrit-Reviewer: Holger Freyther <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <[email protected]> Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-HasComments: Yes
