flac dev - Feb 2012 - Git branch with compiling fixes for win32

On 2/1/2012 18:52, Erik de Castro Lopo wrote:
> JonY wrote:
> 
>> Alright, here's a quick fix, although it is more ugly than I
remembered.
>>
>> Basically, it removes those _MSC_VER ifdefs, and relies on inttypes.h
>> where available, and falls back to I64 on MSVC and then ll for others,
>> all format warnings suppressed.
> 
> JonY,
> 
> Sorry for the delay on actually getting on to this.
> 
> I tried your patch, but it wasn't quite right. The problem is that %ll
> is the correct format specifier for uint64_t on 32bit Linux but not
> on 64 bit Linux.
> 
Something is very very wrong about the above statements. I thought I
used PRI?64 when inttypes.h is found (Linux should have it, old code
uses %ll? anyway for non-msvc, so shouldn't have any new issues).

inttypes.h and stdint.h is supposed to be abstractions to remove these
issues.
> In C99, the correct way to print a uint64_t value is:
> 
>      printf ("THe value is : " PRIu64 "\n", value) ;
> 
> I have gone ahead and fixed this through the code and fixed this, but I
> may have broken some either MSVC or MinGW on the way. I'd appreciate it
> if you test whats in git now.
> 
> For those files where any compiler baulks at the PRIu64/PRIx64/PRId64,
> you should add a #ifdef block that might look something like:
> 
>     #ifdef HAVE_INTTYPES_H
>     #include <inttypes.h>
>     #else
>     #if defined (_MSC_VER) && ! defined (PRId64)
>     #define PRId64 "I64d"
>     #endif
>     #if defined (_MSC_VER) && ! defined (PRIu64)
>     #define PRIu64 "I64u"
>     #endif
>     #endif
> 
> We'll worry about compilers that don't have the PRI_64 values as we
find
> them.
> 
OK, I'll do a quick test build tomorrow. More thorough testing will come
during the weekends.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 196 bytes
Desc: OpenPGP digital signature
Url :
http://lists.xiph.org/pipermail/flac-dev/attachments/20120201/f869625c/attachment.pgp

JonY wrote:
> > Sorry for the delay on actually getting on to this.
> > 
> > I tried your patch, but it wasn't quite right. The problem is that
%ll
> > is the correct format specifier for uint64_t on 32bit Linux but not
> > on 64 bit Linux.
> > 
> 
> Something is very very wrong about the above statements. I thought I
> used PRI?64 when inttypes.h is found (Linux should have it, old code
> uses %ll? anyway for non-msvc, so shouldn't have any new issues).
I think that may have been because configure was not detecting
<inttypes.h> when I first tested that patch.
> inttypes.h and stdint.h is supposed to be abstractions to remove these
> issues.
For the <inttypes.h> and <stdint.h> problem the way I usually prefer
to tackle it is:

   a) Assume they are there.
   b) When they aren't, add the required #ifdef/#define nonsense to
      define them for the compiler when they are missing.
> OK, I'll do a quick test build tomorrow. More thorough testing will
come
> during the weekends.
Cool, thanks.

Erik
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/

On 2/1/2012 22:36, JonY wrote:
> On 2/1/2012 18:52, Erik de Castro Lopo wrote:
>> JonY wrote:
>>
>>> Alright, here's a quick fix, although it is more ugly than I
remembered.
>>>
>>> Basically, it removes those _MSC_VER ifdefs, and relies on
inttypes.h
>>> where available, and falls back to I64 on MSVC and then ll for
others,
>>> all format warnings suppressed.
>>
>> JonY,
>>
>> Sorry for the delay on actually getting on to this.
>>
>> I tried your patch, but it wasn't quite right. The problem is that
%ll
>> is the correct format specifier for uint64_t on 32bit Linux but not
>> on 64 bit Linux.
>>
> 
> Something is very very wrong about the above statements. I thought I
> used PRI?64 when inttypes.h is found (Linux should have it, old code
> uses %ll? anyway for non-msvc, so shouldn't have any new issues).
> 
> inttypes.h and stdint.h is supposed to be abstractions to remove these
> issues.
> 
>> In C99, the correct way to print a uint64_t value is:
>>
>>      printf ("THe value is : " PRIu64 "\n", value)
;
>>
>> I have gone ahead and fixed this through the code and fixed this, but I
>> may have broken some either MSVC or MinGW on the way. I'd
appreciate it
>> if you test whats in git now.
>>
>> For those files where any compiler baulks at the PRIu64/PRIx64/PRId64,
>> you should add a #ifdef block that might look something like:
>>
>>     #ifdef HAVE_INTTYPES_H
>>     #include <inttypes.h>
>>     #else
>>     #if defined (_MSC_VER) && ! defined (PRId64)
>>     #define PRId64 "I64d"
>>     #endif
>>     #if defined (_MSC_VER) && ! defined (PRIu64)
>>     #define PRIu64 "I64u"
>>     #endif
>>     #endif
>>
>> We'll worry about compilers that don't have the PRI_64 values
as we find
>> them.
>>
> 
> OK, I'll do a quick test build tomorrow. More thorough testing will
come
> during the weekends.
> 
Attached patch builds without any warnings for MinGW.

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: format_fix.txt
Url:
http://lists.xiph.org/pipermail/flac-dev/attachments/20120202/b3722d1b/attachment-0001.txt
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 196 bytes
Desc: OpenPGP digital signature
Url :
http://lists.xiph.org/pipermail/flac-dev/attachments/20120202/b3722d1b/attachment-0001.pgp

JonY wrote:
> Attached patch builds without any warnings for MinGW.
Sorry JonY, that patch does not apply against current git master
which is here:

    https://git.xiph.org/?p=flac.git;a=summary

Specifically I pulled out much of the "#ifdef _MSC_VER" printf stuff
in this commit:

   
https://git.xiph.org/?p=flac.git;a=commit;h=ce8a75134cace056f6c436d54b57bad1a1d93797

For example (damn thats a horrible URL):

   
https://git.xiph.org/?p=flac.git;a=blobdiff;f=examples/c/decode/file/main.c;h=e5138b4f9a08f0cf3e9a26c8ab68ac76d161785d;hp=32b555947d5c576693dbb6a9ee13a0a67582d96a;hb=ce8a75134cace056f6c436d54b57bad1a1d93797;hpb=8bbbf56403808ff75126cd0840a936aedbc4113b

Since your patch seems pretty similar to what I have already commited,
I wouldn't be surprised it what we have now doesn't just compile
correctly
under MinGW.

Cheers,
Erik
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/