[PATCH] ussd: Fix text of RELEASE COMPLETE
Before the assigned value (0xFF) was truncated, reg->text[0] is of type char. A corresponding test for the same value in openbsc could only fail. --- src/gsm/gsm0480.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gsm/gsm0480.c b/src/gsm/gsm0480.c index 92a62dc..dbacefc 100644 --- a/src/gsm/gsm0480.c +++ b/src/gsm/gsm0480.c @@ -234,7 +234,7 @@ static int parse_ussd(const struct gsm48_hdr *hdr, uint16_t len, struct ussd_req case GSM0480_MTYPE_RELEASE_COMPLETE: LOGP(0, LOGL_DEBUG, "USS Release Complete\n"); /* could also parse out the optional Cause/Facility data */ - req->text[0] = 0xFF; + req->text[0] = '\0'; break; case GSM0480_MTYPE_REGISTER: case GSM0480_MTYPE_FACILITY: -- 1.8.3.2
On Sun, Oct 6, 2013 at 9:55 PM, Alexander Huemer <alexander.huemer@xx.vu> wrote:
LOGP(0, LOGL_DEBUG, "USS Release Complete\n");
Should this be "USSD" instead of "USS" as well? -- Regards, Alexander Chemeris. CEO, Fairwaves LLC / ООО УмРадио http://fairwaves.ru
On Sun, Oct 06, 2013 at 10:01:26PM +0200, Alexander Chemeris wrote:
On Sun, Oct 6, 2013 at 9:55 PM, Alexander Huemer <alexander.huemer@xx.vu> wrote:
LOGP(0, LOGL_DEBUG, "USS Release Complete\n");
Should this be "USSD" instead of "USS" as well?
I don't know. Opinions? Kind regards, -Alex
On Sun, Oct 06, 2013 at 09:55:09PM +0200, Alexander Huemer wrote:
Before the assigned value (0xFF) was truncated, reg->text[0] is of type char. A corresponding test for the same value in openbsc could only fail.
Can you please explain? signed char a = 0xFF; signed char b = 0xFF; a == b => true. Even if the numerical is not the one, one expected?
- req->text[0] = 0xFF; + req->text[0] = '\0';
but yes, we can null terminate it.
On Tue, Oct 08, 2013 at 11:27:49AM +0200, Holger Hans Peter Freyther wrote:
On Sun, Oct 06, 2013 at 09:55:09PM +0200, Alexander Huemer wrote:
Before the assigned value (0xFF) was truncated, reg->text[0] is of type char. A corresponding test for the same value in openbsc could only fail.
Can you please explain?
char is an signed 8bit type, so the maximum value is 0x7F. Well, at least usually. As I read, ANSI C does not dictate whether a variable declared as 'char' is signed or unsigned, gcc though defaults to signed. Excerpt from limits.h: [...] # define SCHAR_MAX 127 [...] # ifdef __CHAR_UNSIGNED__ (this is not the case normally) [...] # define CHAR_MAX SCHAR_MAX [...] Example program: int main(void) { char c = 0xFF; if (c == 0xFF) return 0; return 1; } gcc gives a hint when -Wtype-limits is used. $ gcc -Wtype-limits main.c $ ./a.out main.c: In function ‘main’: main.c:5:2: warning: comparison is always false due to limited range of data type [-Wtype-limits] $
signed char a = 0xFF; signed char b = 0xFF;
a == b => true. Even if the numerical is not the one, one expected?
This is true because _both_ variables got the same truncated literal. Kind regards, -Alex
On 10/08/2013 11:51 AM, Alexander Huemer wrote:
On Tue, Oct 08, 2013 at 11:27:49AM +0200, Holger Hans Peter Freyther wrote:
On Sun, Oct 06, 2013 at 09:55:09PM +0200, Alexander Huemer wrote:
Before the assigned value (0xFF) was truncated, reg->text[0] is of type char. A corresponding test for the same value in openbsc could only fail.
Can you please explain?
char is an signed 8bit type, so the maximum value is 0x7F. Well, at least usually. As I read, ANSI C does not dictate whether a variable declared as 'char' is signed or unsigned, gcc though defaults to signed. Excerpt from limits.h:
[...] # define SCHAR_MAX 127 [...] # ifdef __CHAR_UNSIGNED__ (this is not the case normally) [...] # define CHAR_MAX SCHAR_MAX [...]
Example program:
int main(void) { char c = 0xFF;
if (c == 0xFF) return 0;
return 1; } And this is because:
char c = 0xff; c == 0xff; ^^ ^^ | an int | promoted to int and sign extended in this expression So it's really -1 == 255 c == (char)0xff; would be ok though
On Tue, Oct 08, 2013 at 11:51:42AM +0200, Alexander Huemer wrote:
On Tue, Oct 08, 2013 at 11:27:49AM +0200, Holger Hans Peter Freyther wrote:
On Sun, Oct 06, 2013 at 09:55:09PM +0200, Alexander Huemer wrote:
Before the assigned value (0xFF) was truncated, reg->text[0] is of type char. A corresponding test for the same value in openbsc could only fail.
Can you please explain?
char is an signed 8bit type, so the maximum value is 0x7F. Well, at least usually. As I read, ANSI C does not dictate whether a variable declared as 'char' is signed or unsigned, gcc though defaults to signed. Excerpt from limits.h:
[...] # define SCHAR_MAX 127 [...] # ifdef __CHAR_UNSIGNED__ (this is not the case normally) [...] # define CHAR_MAX SCHAR_MAX [...]
Example program:
int main(void) { char c = 0xFF;
if (c == 0xFF) return 0;
return 1; }
gcc gives a hint when -Wtype-limits is used.
$ gcc -Wtype-limits main.c $ ./a.out main.c: In function ‘main’: main.c:5:2: warning: comparison is always false due to limited range of data type [-Wtype-limits] $
signed char a = 0xFF; signed char b = 0xFF;
a == b => true. Even if the numerical is not the one, one expected?
This is true because _both_ variables got the same truncated literal.
A week has passed since the last comment on this (and my other) patch. What is the current state? Are the two patches rejected because they do not make sense? Should I modify anything? Will they be merged at some point in the future? Kind regards, -Alex
On Tue, Oct 15, 2013 at 11:44:04AM +0200, Alexander Huemer wrote:
A week has passed since the last comment on this (and my other) patch. What is the current state? Are the two patches rejected because they do not make sense? Should I modify anything? Will they be merged at some point in the future?
Thanks, I will merge them.
participants (4)
-
Alexander Chemeris -
Alexander Huemer -
Holger Hans Peter Freyther -
noselasd＠fiane.dyndns.org