bugFreeciv - Bugs: bug #20513, dataio.c compiler warnings

 
 
Show feedback again

bug #20513: dataio.c compiler warnings

Submitted by:  Marko Lindqvist <cazfi>
Submitted on:  Fri 15 Feb 2013 01:59:12 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Priority: 9 - ImmediateStatus: Fixed
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Release: Operating System: None
Planned Release: 2.5.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sun 17 Feb 2013 01:35:46 PM UTC, comment #12:

For the record:

> But I thought that sizeof(long) were equal to 8 bytes on 64 bits
> computers.

Not on "LLP64" platforms... there's a useful taxonomy of 64-bit data models on Wikipedia.

Jacob Nevins <jtn>
Project Administrator
Fri 15 Feb 2013 10:02:54 PM UTC, SVN revision 22342:

Avoid problems with printf format for size_t value by hardcoding
the value. Assert that value is correct.

See gna bug #20513

(Browse SVN revision 22342)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Fri 15 Feb 2013 10:00:39 PM UTC, comment #10:

I made a version that moves the asserts before this, and turns them to FC_STATIC_ASSERT.

I'm going to commit soon.

(file #17207)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Fri 15 Feb 2013 06:25:24 PM UTC, comment #9:

New patch attached which doesn't use the c-format (hard-coded strings).

(file #17206)

pepeto <pepeto>
Project Member
Fri 15 Feb 2013 03:36:47 PM UTC, comment #8:

> It can hold uint32_t, but not pointer or size_t on win64.


I am sorry for the noise I caused... But I thought that sizeof(long) were equal to 8 bytes on 64 bits computers.

pepeto <pepeto>
Project Member
Fri 15 Feb 2013 03:27:06 PM UTC, comment #7:

> The cast to (unsigned long) is used a bit everywhere in the
> code..


It can hold uint32_t, but not pointer or size_t on win64.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Fri 15 Feb 2013 03:03:34 PM UTC, comment #6:

The cast to (unsigned long) is used a bit everywhere in the code...

> We can/should move the fc_assert(sizeof(x) == 4) earlier in the
> function, before one tries to read 4 bytes to sizeof(x) block.


Yes. And it could also be replaced by FC_STATIC_ASSERT() here I guess.

pepeto <pepeto>
Project Member
Fri 15 Feb 2013 02:59:07 PM UTC, comment #5:

This is proving harder than I expected

- My MinGW compiler does not support "%zd" (C99 feature)
- We cannot rely on inttypes.h to exist, nor has it format identifier that is guaranteed to match site_t on all platforms
- Casting size_t to unsigned long wouldn't work on win64 (we recently got that otherwise supported) as there size_t is bigger than long

But myabe we are trying to fix wrong error here. Why those messages even are like that? Look for dio_get_uint32(): It tries to read 4 bytes, and when it fails, it tries to figure out number of bytes it tried to read with sizeof(). The same value should be used for both reading and to be in log message. Either we get it to variable and use that variable, or we use same hardcoded value for both. Given the problem in this ticket, we should go by hardcoding. We can/should move the fc_assert(sizeof(x) == 4) earlier in the function, before one tries to read 4 bytes to sizeof(x) block.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Fri 15 Feb 2013 02:11:52 PM UTC, comment #4:

Casts are no good! It just prevents the warning, but not the bug of using another size variable than what there is memory for.

Proper fix will take me a couple of moments...

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Fri 15 Feb 2013 02:10:44 PM UTC, comment #3:

This one works for me.

(file #17204)

pepeto <pepeto>
Project Member
Fri 15 Feb 2013 02:08:29 PM UTC, comment #2:

It doesn't for me. I will add casts...

dataio.c: In function 'dio_get_uint8':
dataio.c:501:5: error: format '%ld' expects argument of type 'long int', but argument 7 has type 'unsigned int' [-Werror=format]
dataio.c: In function 'dio_get_uint16':
dataio.c:521:5: error: format '%ld' expects argument of type 'long int', but argument 7 has type 'unsigned int' [-Werror=format]
dataio.c: In function 'dio_get_uint32':
dataio.c:541:5: error: format '%ld' expects argument of type 'long int', but argument 7 has type 'unsigned int' [-Werror=format]

pepeto <pepeto>
Project Member
Fri 15 Feb 2013 02:06:13 PM UTC, comment #1:

This fixes it for me.

(file #17202)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Fri 15 Feb 2013 01:59:12 PM UTC, original submission:

As of r22341 --enable-debug (-Werror) build fails:

common/dataio.c: In function 'dio_get_uint8':
common/dataio.c:501:5: error: format '%d' expects argument of type 'int', but argument 7 has type 'long unsigned int' [-Werror=format]
common/dataio.c: In function 'dio_get_uint16':
common/dataio.c:520:5: error: format '%d' expects argument of type 'int', but argument 7 has type 'long unsigned int' [-Werror=format]
common/dataio.c: In function 'dio_get_uint32':
common/dataio.c:539:5: error: format '%d' expects argument of type 'int', but argument 7 has type 'long unsigned int' [-Werror=format]

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #17207:  SizeTPrintfNo.patch added by cazfi (1kB - text/x-diff)
file #17206:  dio_get_uintxxx.diff added by pepeto (913B - text/x-patch)
file #17204:  sizeof_cast.diff added by pepeto (1kB - text/x-diff)
file #17202:  SizeofPrintfFormat.patch added by cazfi (883B - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jtn (Posted a comment)
  • -unavailable- added by pepeto (Posted a comment)
  • -unavailable- added by cazfi (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 15 Feb 2013 10:03:13 PM UTCcazfiStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Fri 15 Feb 2013 10:00:39 PM UTCcazfiAttached File-=>Added SizeTPrintfNo.patch, #17207
      Assigned toNone=>cazfi
    Fri 15 Feb 2013 06:25:24 PM UTCpepetoAttached File-=>Added dio_get_uintxxx.diff, #17206
    Fri 15 Feb 2013 02:10:44 PM UTCpepetoAttached File-=>Added sizeof_cast.diff, #17204
    Fri 15 Feb 2013 02:06:13 PM UTCcazfiAttached File-=>Added SizeofPrintfFormat.patch, #17202
      Priority5 - Normal=>9 - Immediate
      StatusNone=>Ready For Test
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup