Attention is currently required from: jolly.

laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/34985?usp=email )

Change subject: LAPD: Add support for RTS based polling and T200
......................................................................


Patch Set 5:

(3 comments)

File include/osmocom/isdn/lapd_core.h:

https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/d3629140_d95fb0c8
PS5, Line 155:  unsigned
you are modifying a public data structure, which is ABI breakage.  Old 
applications must not link against new lib and vice-versa.  This must be 
documented in TODO-RELEASE as it requires a libversion bump.


File src/isdn/lapd_core.c:

https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/72c0d46c_161a9fcd
PS5, Line 220: /
handle how? This requires more ddocumentation.  I know the original code is not 
very good in terms of doxygen aPI documentation, but new functions could start 
with it right away.

The library API must document how it is used by an application program.


https://gerrit.osmocom.org/c/libosmocore/+/34985/comment/50e7c2ca_89a4d743
PS5, Line 399:  dl->flags = flags;
I guess it would leads to undefined behaviour if somebody called 
lapd_dl_set_flags to enable RTS mode at a random *later* time after already 
having started to use the non-RTS mode.  We should protect against this here.  
I guess the RTS flag should only be modifiable during an initial state before 
any timers are started?



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34985?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ib961b5a44911b99b0487641533301749c0286995
Gerrit-Change-Number: 34985
Gerrit-PatchSet: 5
Gerrit-Owner: jolly <andr...@eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <lafo...@osmocom.org>
Gerrit-Attention: jolly <andr...@eversberg.eu>
Gerrit-Comment-Date: Mon, 13 Nov 2023 18:24:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to