Discussion:
[PATCH] Fix bunch of printf warnings
Max
2014-10-22 14:50:08 UTC
Permalink
Signed-off-by: Max <***@fairwaves.co>
---
openbsc/src/libbsc/bsc_api.c | 4 ++--
openbsc/src/libmgcp/mgcp_protocol.c | 2 +-
openbsc/tests/abis/abis_test.c | 16 ++++++++--------
openbsc/tests/gbproxy/gbproxy_test.c | 16 ++++++++--------
4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/openbsc/src/libbsc/bsc_api.c b/openbsc/src/libbsc/bsc_api.c
index 5ce0bc4..f4121de 100644
--- a/openbsc/src/libbsc/bsc_api.c
+++ b/openbsc/src/libbsc/bsc_api.c
@@ -416,7 +416,7 @@ static void handle_ass_compl(struct gsm_subscriber_connection *conn,

gh = msgb_l3(msg);
if (msgb_l3len(msg) - sizeof(*gh) != 1) {
- LOGP(DMSC, LOGL_ERROR, "Assignment Compl invalid: %u\n",
+ LOGP(DMSC, LOGL_ERROR, "Assignment Compl invalid: %lu\n",
msgb_l3len(msg) - sizeof(*gh));
return;
}
@@ -461,7 +461,7 @@ static void handle_ass_fail(struct gsm_subscriber_connection *conn,

gh = msgb_l3(msg);
if (msgb_l3len(msg) - sizeof(*gh) != 1) {
- LOGP(DMSC, LOGL_ERROR, "assignemnt failure unhandled: %u\n",
+ LOGP(DMSC, LOGL_ERROR, "assignemnt failure unhandled: %lu\n",
msgb_l3len(msg) - sizeof(*gh));
rr_failure = NULL;
} else {
diff --git a/openbsc/src/libmgcp/mgcp_protocol.c b/openbsc/src/libmgcp/mgcp_protocol.c
index 79422fe..00315fb 100644
--- a/openbsc/src/libmgcp/mgcp_protocol.c
+++ b/openbsc/src/libmgcp/mgcp_protocol.c
@@ -308,7 +308,7 @@ static int write_response_sdp(struct mgcp_endpoint *endp,
return len;

buffer_too_small:
- LOGP(DMGCP, LOGL_ERROR, "SDP buffer too small: %d (needed %d)\n",
+ LOGP(DMGCP, LOGL_ERROR, "SDP buffer too small: %zu (needed %d)\n",
size, len);
return -1;
}
diff --git a/openbsc/tests/abis/abis_test.c b/openbsc/tests/abis/abis_test.c
index e7e78d2..f82740f 100644
--- a/openbsc/tests/abis/abis_test.c
+++ b/openbsc/tests/abis/abis_test.c
@@ -61,11 +61,11 @@ static void test_simple_sw_config(void)
}

if (descr[0].len != 13) {
- printf("WRONG SIZE: %d\n", descr[0].len);
+ printf("WRONG SIZE: %zu\n", descr[0].len);
abort();
}

- printf("Start: %u len: %zu\n", descr[0].start - simple_config, descr[0].len);
+ printf("Start: %lu len: %zu\n", descr[0].start - simple_config, descr[0].len);
printf("file_id: %s\n", osmo_hexdump(descr[0].file_id, descr[0].file_id_len));
printf("file_ver: %s\n", osmo_hexdump(descr[0].file_ver, descr[0].file_ver_len));
}
@@ -99,20 +99,20 @@ static void test_dual_sw_config(void)
}

if (descr[0].len != 13) {
- printf("WRONG SIZE0: %d\n", descr[0].len);
+ printf("WRONG SIZE0: %zu\n", descr[0].len);
abort();
}

if (descr[1].len != 13) {
- printf("WRONG SIZE1: %d\n", descr[1].len);
+ printf("WRONG SIZE1: %zu\n", descr[1].len);
abort();
}

- printf("Start: %u len: %zu\n", descr[0].start - dual_config, descr[0].len);
+ printf("Start: %lu len: %zu\n", descr[0].start - dual_config, descr[0].len);
printf("file_id: %s\n", osmo_hexdump(descr[0].file_id, descr[0].file_id_len));
printf("file_ver: %s\n", osmo_hexdump(descr[0].file_ver, descr[0].file_ver_len));

- printf("Start: %u len: %zu\n", descr[1].start - dual_config, descr[1].len);
+ printf("Start: %lu len: %zu\n", descr[1].start - dual_config, descr[1].len);
printf("file_id: %s\n", osmo_hexdump(descr[1].file_id, descr[1].file_id_len));
printf("file_ver: %s\n", osmo_hexdump(descr[1].file_ver, descr[1].file_ver_len));
}
@@ -129,11 +129,11 @@ static void test_sw_selection(void)
abort();
}

- printf("Start: %u len: %zu\n", descr[0].start - load_config, descr[0].len);
+ printf("Start: %lu len: %zu\n", descr[0].start - load_config, descr[0].len);
printf("file_id: %s\n", osmo_hexdump(descr[0].file_id, descr[0].file_id_len));
printf("file_ver: %s\n", osmo_hexdump(descr[0].file_ver, descr[0].file_ver_len));

- printf("Start: %u len: %zu\n", descr[1].start - load_config, descr[1].len);
+ printf("Start: %lu len: %zu\n", descr[1].start - load_config, descr[1].len);
printf("file_id: %s\n", osmo_hexdump(descr[1].file_id, descr[1].file_id_len));
printf("file_ver: %s\n", osmo_hexdump(descr[1].file_ver, descr[1].file_ver_len));

diff --git a/openbsc/tests/gbproxy/gbproxy_test.c b/openbsc/tests/gbproxy/gbproxy_test.c
index 5aa301b..161f7fd 100644
--- a/openbsc/tests/gbproxy/gbproxy_test.c
+++ b/openbsc/tests/gbproxy/gbproxy_test.c
@@ -898,7 +898,7 @@ int gprs_ns_rcvmsg(struct gprs_ns_inst *nsi, struct msgb *msg,
int gprs_ns_callback(enum gprs_ns_evt event, struct gprs_nsvc *nsvc,
struct msgb *msg, uint16_t bvci)
{
- printf("CALLBACK, event %d, msg length %d, bvci 0x%04x\n%s\n\n",
+ printf("CALLBACK, event %d, msg length %zu, bvci 0x%04x\n%s\n\n",
event, msgb_bssgp_len(msg), bvci,
osmo_hexdump(msgb_l2(msg), msgb_l2len(msg)));

@@ -925,15 +925,15 @@ ssize_t sendto(int sockfd, const void *buf, size_t len, int flags,
real_sendto = dlsym(RTLD_NEXT, "sendto");

if (dest_host == REMOTE_BSS_ADDR)
- printf("MESSAGE to BSS at 0x%08x:%d, msg length %d\n%s\n\n",
+ printf("MESSAGE to BSS at 0x%08x:%d, msg length %zu\n%s\n\n",
dest_host, dest_port,
len, osmo_hexdump(buf, len));
else if (dest_host == REMOTE_SGSN_ADDR)
- printf("MESSAGE to SGSN at 0x%08x:%d, msg length %d\n%s\n\n",
+ printf("MESSAGE to SGSN at 0x%08x:%d, msg length %zu\n%s\n\n",
dest_host, dest_port,
len, osmo_hexdump(buf, len));
else if (dest_host == REMOTE_SGSN2_ADDR)
- printf("MESSAGE to SGSN 2 at 0x%08x:%d, msg length %d\n%s\n\n",
+ printf("MESSAGE to SGSN 2 at 0x%08x:%d, msg length %zu\n%s\n\n",
dest_host, dest_port,
len, osmo_hexdump(buf, len));
else
@@ -957,15 +957,15 @@ int gprs_ns_sendmsg(struct gprs_ns_inst *nsi, struct msgb *msg)

if (nsei == SGSN_NSEI)
printf("NS UNITDATA MESSAGE to SGSN, BVCI 0x%04x, "
- "msg length %d (%s)\n",
+ "msg length %zu (%s)\n",
bvci, len, __func__);
else if (nsei == SGSN2_NSEI)
printf("NS UNITDATA MESSAGE to SGSN 2, BVCI 0x%04x, "
- "msg length %d (%s)\n",
+ "msg length %zu (%s)\n",
bvci, len, __func__);
else
printf("NS UNITDATA MESSAGE to BSS, BVCI 0x%04x, "
- "msg length %d (%s)\n",
+ "msg length %zu (%s)\n",
bvci, len, __func__);

if (received_messages) {
@@ -1195,7 +1195,7 @@ static int gprs_process_message(struct gprs_ns_inst *nsi, const char *text, stru
struct msgb *msg;
int ret;
if (data_len > NS_ALLOC_SIZE - NS_ALLOC_HEADROOM) {
- fprintf(stderr, "message too long: %d\n", data_len);
+ fprintf(stderr, "message too long: %zu\n", data_len);
return -1;
}
--
1.9.1
2014-10-22 15:15:26 UTC
Permalink
One step closer to build with -Werror :)
--
best regards,
Max, http://fairwaves.co
Holger Hans Peter Freyther
2014-10-22 21:32:49 UTC
Permalink
On Wed, Oct 22, 2014 at 04:50:08PM +0200, Max wrote:

Hi,
Post by Max
gh = msgb_l3(msg);
if (msgb_l3len(msg) - sizeof(*gh) != 1) {
- LOGP(DMSC, LOGL_ERROR, "Assignment Compl invalid: %u\n",
+ LOGP(DMSC, LOGL_ERROR, "Assignment Compl invalid: %lu\n",
msgb_l3len(msg) - sizeof(*gh));
it looks like a candidate for ping-pong games on 32bit/64bit userland
on GNU/Linux. What is the type here? unsigned int - size_t?

holger
2014-10-23 08:24:57 UTC
Permalink
Good catch, I didn't notice it. Maybe we should just use size_t for all the length
functions in libosmo*? That's what size_t was invented for after all :)
Post by Holger Hans Peter Freyther
Hi,
Post by Max
gh = msgb_l3(msg);
if (msgb_l3len(msg) - sizeof(*gh) != 1) {
- LOGP(DMSC, LOGL_ERROR, "Assignment Compl invalid: %u\n",
+ LOGP(DMSC, LOGL_ERROR, "Assignment Compl invalid: %lu\n",
msgb_l3len(msg) - sizeof(*gh));
it looks like a candidate for ping-pong games on 32bit/64bit userland
on GNU/Linux. What is the type here? unsigned int - size_t?
holger
--
best regards,
Max, http://fairwaves.co
Holger Hans Peter Freyther
2014-10-23 12:39:21 UTC
Permalink
Post by ☎
Good catch, I didn't notice it. Maybe we should just use size_t for all the length
functions in libosmo*? That's what size_t was invented for after all :)
Post by Holger Hans Peter Freyther
it looks like a candidate for ping-pong games on 32bit/64bit userland
on GNU/Linux. What is the type here? unsigned int - size_t?
I can't quote the ISO C specification. So what type is the result of
"unsigned int - size_t"? We have size_t, off_t, ptrdiff_t as potential
types. I don't know which is the right one. :)
Jan Engelhardt
2014-10-23 12:47:26 UTC
Permalink
Post by Holger Hans Peter Freyther
Post by ☎
Good catch, I didn't notice it. Maybe we should just use size_t for all the length
functions in libosmo*? That's what size_t was invented for after all :)
Post by Holger Hans Peter Freyther
it looks like a candidate for ping-pong games on 32bit/64bit userland
on GNU/Linux. What is the type here? unsigned int - size_t?
I can't quote the ISO C specification. So what type is the result of
"unsigned int - size_t"?
size_t, because it is of higher rank.
(The specifier to use is %zu.)
Peter Stuge
2014-10-23 15:59:32 UTC
Permalink
Post by Jan Engelhardt
So what type is the result of "unsigned int - size_t"?
size_t, because it is of higher rank.
How come it has higher rank?

I don't have the spec, maybe it is stated explicitly there, but this
blog post (it's on the internet so it MUST be true!) outlines rules
based on the standard, and doesn't mention either size_t type.

I have a feeling that size_t is not strictly defined, ie. that a
programmer or even a compiler is free to choose any representation,
as long as all memory addresses can be stored. In that case size_t
rank can't really be determined.

We could always cast... :)


//Peter
2014-10-23 16:09:51 UTC
Permalink
Post by Peter Stuge
We could always cast... :)
That's an evil witchcraft! :)
--
best regards,
Max, http://fairwaves.co
Holger Hans Peter Freyther
2014-10-25 13:42:40 UTC
Permalink
Post by Peter Stuge
How come it has higher rank?
I don't have the spec, maybe it is stated explicitly there, but this
blog post (it's on the internet so it MUST be true!) outlines rules
based on the standard, and doesn't mention either size_t type.
#include <stdint.h>
#include <sys/socket.h>
#include <stdint.h>

int foo()
{
unsigned int foo = 1212323123123;
size_t bla = 2;

typeof(foo - bla) k;
k = 3;
}


clang-3.4 -Xclang -ast-dump -fsyntax-only foo.c

`-FunctionDecl 0x9946300 </tmp/foo.c:5:1, line:12:1> foo 'int ()'
`-CompoundStmt 0x9946588 <line:6:1, line:12:1>
|-DeclStmt 0x99463c8 <line:7:2, col:34>
| `-VarDecl 0x9946370 <col:2, col:21> foo 'unsigned int'
| `-ImplicitCastExpr 0x99463b8 <col:21> 'unsigned int' <IntegralCast>
| `-IntegerLiteral 0x99463a0 <col:21> 'long long' 1212323123123
|-DeclStmt 0x9946438 <line:8:2, col:16>
| `-VarDecl 0x99463e0 <col:2, col:15> bla 'size_t':'unsigned int'
| `-ImplicitCastExpr 0x9946428 <col:15> 'size_t':'unsigned int' <IntegralCast>
| `-IntegerLiteral 0x9946410 <col:15> 'int' 2
|-DeclStmt 0x9946520 <line:10:2, col:21>
| `-VarDecl 0x99464f0 <col:2, col:20> k 'typeof (foo - bla)':'unsigned int'
`-BinaryOperator 0x9946570 <line:11:2, col:6> 'typeof (foo - bla)':'unsigned int' '='
|-DeclRefExpr 0x9946530 <col:2> 'typeof (foo - bla)':'unsigned int' lvalue Var 0x99464f0 'k' 'typeof (foo - bla)':'unsigned int'
`-ImplicitCastExpr 0x9946560 <col:6> 'typeof (foo - bla)':'unsigned int' <IntegralCast>
`-IntegerLiteral 0x9946548 <col:6> 'int' 3

If I read this correctly the type is 'unsigned int' with clang? An
issue with my reading? Clang bug?


cheers
holger
Jan Engelhardt
2014-10-25 15:41:12 UTC
Permalink
Post by Holger Hans Peter Freyther
int foo()
{
unsigned int foo = 1212323123123;
size_t bla = 2;
typeof(foo - bla) k;
k = 3;
}
clang-3.4 -Xclang -ast-dump -fsyntax-only foo.c
| `-VarDecl 0x99464f0 <col:2, col:20> k 'typeof (foo - bla)':'unsigned int'
If I read this correctly the type is 'unsigned int' with clang?
You seem to be compiling in LP32 mode.
I get unsigned long -- which is what size_t amounts to on Linux/LP64.
Peter Stuge
2014-10-26 12:04:36 UTC
Permalink
Post by Peter Stuge
Post by Jan Engelhardt
So what type is the result of "unsigned int - size_t"?
size_t, because it is of higher rank.
How come it has higher rank?
I'd appreciate an answer to this. "because" isn't it.


//Peter
Jan Engelhardt
2014-10-26 17:29:21 UTC
Permalink
Post by Peter Stuge
Post by Peter Stuge
Post by Jan Engelhardt
So what type is the result of "unsigned int - size_t"?
size_t, because it is of higher rank.
How come it has higher rank?
I'd appreciate an answer to this. "because" isn't it.
Then you will be disappointed, because "because it is" is the answer -
the standards body defined it to be that way.
https://www.securecoding.cert.org/confluence/display/seccode/INT02-C.+Understand+integer+conversion+rules
Peter Stuge
2014-10-27 07:41:16 UTC
Permalink
Post by Jan Engelhardt
Post by Peter Stuge
Post by Peter Stuge
Post by Jan Engelhardt
So what type is the result of "unsigned int - size_t"?
size_t, because it is of higher rank.
How come it has higher rank?
I'd appreciate an answer to this. "because" isn't it.
"because it is" is the answer -
the standards body defined it to be that way.
It's not clear to me that this is the case.
Post by Jan Engelhardt
https://www.securecoding.cert.org/confluence/display/seccode/INT02-C.+Understand+integer+conversion+rules
Right, this is the blog post I referenced in my previous email. I
forgot to include the link. Thanks for sending it.

This post makes a reference to the standard and seems to repeat the
rank rules, but it doesn't seem to me that any of the rules apply to
the case at hand?


//Peter

Loading...