Patch Set 1: Code-Review-2

(8 comments)

Hi Muhammad. Thanks for your submission.  However, I had to stop after 
reviewing about one third of the patch, as it seems you have not been using git 
properly and the patch is removing lots of code from current master. Please 
make sure to submit a clean patch on top of current master, which only adsd the 
intended changes without changing any other parts or even removing features 
like LUA. Thanks!

https://gerrit.osmocom.org/#/c/5490/1/src/host/layer23/src/common/l1ctl.c
File src/host/layer23/src/common/l1ctl.c:

Line 24: //MTZ
please don't submit changes like this (//MTZ)


Line 62: int count_test1=0;
where is this variable used?


PS1, Line 164: in
we have an elaborate logging system and don't use printf. same applies for all 
similar printf() calls below


https://gerrit.osmocom.org/#/c/5490/1/src/host/layer23/src/common/logging.c
File src/host/layer23/src/common/logging.c:

Line 130
why were those removed? Maybe you need to rebase your patch on top of latest 
master?


https://gerrit.osmocom.org/#/c/5490/1/src/host/layer23/src/common/main.c
File src/host/layer23/src/common/main.c:

Line 249:       sprintf(ms->name, "1");
unrelated change


https://gerrit.osmocom.org/#/c/5490/1/src/host/layer23/src/common/sim.c
File src/host/layer23/src/common/sim.c:

Line 32: extern void *l23_ctx;
why not keep 'ms' as talloc context?


https://gerrit.osmocom.org/#/c/5490/1/src/host/layer23/src/mobile/Makefile.am
File src/host/layer23/src/mobile/Makefile.am:

Line 3
why are you removing LUA support. Please stop doing that.


https://gerrit.osmocom.org/#/c/5490/1/src/host/layer23/src/mobile/gsm322.c
File src/host/layer23/src/mobile/gsm322.c:

Line 5124:      char filename[PATH_MAX];
unrelated change


-- 
To view, visit https://gerrit.osmocom.org/5490
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib01460b796d2107c4599d327e184eb42340999d2
Gerrit-PatchSet: 1
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Owner: Muhammad Awais Aslam <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <[email protected]>
Gerrit-Reviewer: Neels Hofmeyr <[email protected]>
Gerrit-Reviewer: Vadim Yanitskiy <[email protected]>
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-HasComments: Yes

Reply via email to