A correcsponding change in libosmocore sets text[0] to '\0'. The test for 0xFF could never have been true. --- openbsc/src/libmsc/ussd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/openbsc/src/libmsc/ussd.c b/openbsc/src/libmsc/ussd.c index 76ee101..b413c83 100644 --- a/openbsc/src/libmsc/ussd.c +++ b/openbsc/src/libmsc/ussd.c @@ -51,7 +51,7 @@ int handle_rcv_ussd(struct gsm_subscriber_connection *conn, struct msgb *msg) memset(&req, 0, sizeof(req)); gh = msgb_l3(msg); rc = gsm0480_decode_ussd_request(gh, msgb_l3len(msg), &req); - if (req.text[0] == 0xFF) /* Release-Complete */ + if (req.text[0] == '\0') /* Release-Complete */ return 0;
if (!strcmp(USSD_TEXT_OWN_NUMBER, (const char *)req.text)) {
On Tue, Oct 08, 2013 at 11:28:21AM +0200, Holger Hans Peter Freyther wrote:
On Sun, Oct 06, 2013 at 09:59:28PM +0200, Alexander Huemer wrote:
- if (req.text[0] == 0xFF) /* Release-Complete */
- if (req.text[0] == '\0') /* Release-Complete */ return 0;
as we can't assume to use the latest libosmocore, what about checking for both?
Well, as I see it the test against 0xFF is not true under any circumstances, as long as the code is not intentionally compiled with -funsigned-char. If there is an undesired behavior change because of this code change, it's a bug. What's your opinion?
Kind regards, -Alex
On Tue, Oct 08, 2013 at 11:59:09AM +0200, Alexander Huemer wrote:
Dear Alexander,
Well, as I see it the test against 0xFF is not true under any circumstances, as long as the code is not intentionally compiled with -funsigned-char. If there is an undesired behavior change because of this code change, it's a bug. What's your opinion?
well, I agree that it is not nice and we can/should change it to a plain null termination. What I doubt that it is currently a problem or can you present a smoking gun?
int main(int argc, char **argv) { signed char a = 0xFF; signed char b = 0xFF;
if (a == b) printf("THE SAME\n"); return 0; }
PS: some ABIs.. e.g. ARM ones.. char will be unsigned.
On Tue, Oct 08, 2013 at 12:49:56PM +0200, Holger Hans Peter Freyther wrote:
On Tue, Oct 08, 2013 at 11:59:09AM +0200, Alexander Huemer wrote:
Dear Alexander,
Well, as I see it the test against 0xFF is not true under any circumstances, as long as the code is not intentionally compiled with -funsigned-char. If there is an undesired behavior change because of this code change, it's a bug. What's your opinion?
well, I agree that it is not nice and we can/should change it to a plain null termination. What I doubt that it is currently a problem or can you present a smoking gun?
No smoking gun that I know of. It's just that the interaction between the piece of code in libosmocore and the piece in openbsc cannot work as the author intended.
int main(int argc, char **argv) { signed char a = 0xFF; signed char b = 0xFF;
if (a == b) printf("THE SAME\n"); return 0; }
Yes, that works of course, but I think my example program better illustrates the current situation in libosmocore and openbsc. In libosmocore a value is assigned which is truncated, in openbsc a comparison is made again that same value and this test returns false, though the intension was to return true.
PS: some ABIs.. e.g. ARM ones.. char will be unsigned.
I only tested the patches on x86_64, but '\0' should be safe on all architectures.
Kind regards, -Alex
-
Alexander Huemer -
Holger Hans Peter Freyther