> 42:21AM -0700, D. Starke wrote: > > From: Daniel Starke <daniel.starke@xxxxxxxxxxx> > > > > n_gsm is based on the 3GPP 07.010 and its newer version is the 3GPP 27.010. > > See https://portal.3gpp.org/desktopmodules/Specifications/SpecificationDetails.aspx?specificationId=1516 > > The changes from 07.010 to 27.010 are non-functional. Therefore, I > > refer to the newer 27.010 here. Chapter 5.4.3.1 states the CR bit in > > UI and UIH frames shall always be set 1 by the initiator and 0 by the responder. > > This has nothing to do with the change you made here. > > > > Currently, gsm_queue() has a pre-processor gated (excluded) check > > which treats all frames that conform to the standard as malformed frames. > > Remove this optional code to avoid confusion and possible breaking > > changes in case that someone includes it. > > Again, nothing to do with the code change. Including this code (i.e. with #if 1) will treat every correct UI/UIH frame as invalid, because the cr flag is always set to 1 for those frames (as mentioned in chapter 5.4.3.1 of the standard). This is obviously wrong. > > > > Fixes: e1eaea46bb40 ("tty: n_gsm line discipline") > > This "fixes" nothing :( What is the correct way to handle the removal of such dead and obviously wrong code? > > Cc: stable@xxxxxxxxxxxxxxx > > How is commenting out unused code a stable backport requirement? True, it does not change the behavior but it fixes a commit which is also present in the current stable release. I was unsure how to handle this case. I will remove the backport remark. > > Signed-off-by: Daniel Starke <daniel.starke@xxxxxxxxxxx> > > --- > > drivers/tty/n_gsm.c | 4 ---- > > 1 file changed, 4 deletions(-) > > > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index > > e9a7d9483c1f..f4ec48c0d6d7 100644 > > --- a/drivers/tty/n_gsm.c > > +++ b/drivers/tty/n_gsm.c > > @@ -1896,10 +1896,6 @@ static void gsm_queue(struct gsm_mux *gsm) > > case UI|PF: > > case UIH: > > case UIH|PF: > > -#if 0 > > - if (cr) > > - goto invalid; > > -#endif > > All you are doing is cleaning up dead code. Not a big deal, and not > worth all the text above to confuse people :( As mentioned above, this is not only dead but also wrong code. I tried to elaborate the reason for it being wrong code in the text above. Best regards, Daniel Starke
