Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[lvqcl]

Erik de Castro Lopo wrote:


Not all of them *needed( converting, but afaiac this makes the code
base better. I never liked the way the original C standard allowed
the use of `unsigned` alone as type.


These recent updates broke MSVC 2005 (and probably 2008) builds
because they don't have  and all these [u]intNN_t types.

A possible solution is to move their definition from share/compat.h
into FLAC/ordinals.h. After all, this file already has this ifdef:

#if defined(_MSC_VER) && _MSC_VER < 1600

...and also to revert back src\share\getopt\getopt.c. After all,
its coding style is EXTREMELY old and imho it won't benefit
much from 'unsigned int' -> 'uint32_t' replacement.
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[Erik de Castro Lopo]

lvqcl wrote:

> Ok, Visual Studio doesn't complain if idx is declared as uint32_t.

As expected :).

Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[lvqcl]

Erik de Castro Lopo wrote:


It's synonymous to uint32_t, but if the 1st parameter for
_BitScanReverse*()
functions has 'pointer to unsigned long' type then IMHO it's better to
simply use unsigned long variable there.


Sorry, I disagree on the idea of using `unsigned long` exactly because
anyone that comes from Unix will assume that is `uint64_t`. Better to
use `uint32_t` which is totally platform independant and unambiguous.


Ok, Visual Studio doesn't complain if idx is declared as uint32_t.
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[Erik de Castro Lopo]

lvqcl wrote:

> > Ah, missed that because it was MSVC code. They should be `uint64_t`.
> 
> No, sizeof(unsigned long) is always 4 on Windows.
> See http://www.viva64.com/en/t/0012/

Bah! Trust Windows to be different :).

> >> According to MSDN _BitScanReverse*() functions have signatures:
> >> unsigned char _BitScanReverse(unsigned long *, unsigned long);
> >> unsigned char _BitScanReverse64(unsigned long *, unsigned __int64);
> >
> > The `unsigned long` type should be synonymous with `uint64_t` so using
> > `uint64_t` should be safe. Furthermore if they aren't synonymous we
> > *want* that to be a compile error!
> 
> It's synonymous to uint32_t, but if the 1st parameter for  
> _BitScanReverse*()
> functions has 'pointer to unsigned long' type then IMHO it's better to
> simply use unsigned long variable there.

Sorry, I disagree on the idea of using `unsigned long` exactly because
anyone that comes from Unix will assume that is `uint64_t`. Better to
use `uint32_t` which is totally platform independant and unambiguous.


Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[lvqcl]

Erik de Castro Lopo wrote:


Also MSVC fails because src/libFLAC/include/private/bitmath.h now
contains "uint32_t long idx" instead of "unsigned long idx".


Ah, missed that because it was MSVC code. They should be `uint64_t`.


No, sizeof(unsigned long) is always 4 on Windows.
See http://www.viva64.com/en/t/0012/


According to MSDN _BitScanReverse*() functions have signatures:
unsigned char _BitScanReverse(unsigned long *, unsigned long);
unsigned char _BitScanReverse64(unsigned long *, unsigned __int64);


The `unsigned long` type should be synonymous with `uint64_t` so using
`uint64_t` should be safe. Furthermore if they aren't synonymous we
*want* that to be a compile error!


It's synonymous to uint32_t, but if the 1st parameter for  
_BitScanReverse*()

functions has 'pointer to unsigned long' type then IMHO it's better to
simply use unsigned long variable there.
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[Erik de Castro Lopo]

Ozkan Sezer wrote:

> Well, the commit seems like overkill :)  not all the unsigned needed
> converting.

Not all of them *needed( converting, but afaiac this makes the code
base better. I never liked the way the original C standard allowed
the use of `unsigned` alone as type.

Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[Erik de Castro Lopo]

lvqcl wrote:

> Also MSVC fails because src/libFLAC/include/private/bitmath.h now
> contains "uint32_t long idx" instead of "unsigned long idx".

Ah, missed that because it was MSVC code. They should be `uint64_t`.

> According to MSDN _BitScanReverse*() functions have signatures:
> unsigned char _BitScanReverse(unsigned long *, unsigned long);
> unsigned char _BitScanReverse64(unsigned long *, unsigned __int64);

The `unsigned long` type should be synonymous with `uint64_t` so using
`uint64_t` should be safe. Furthermore if they aren't synonymous we
*want* that to be a compile error!

> Other than that, it compiles everything without errors.

Thanks!

Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[lvqcl]

Ozkan Sezer wrote:


Well, the commit seems like overkill :)  not all the unsigned needed
converting.

And build fails :
format.c:53: error: conflicting types for 'FLAC__STREAM_SYNC'
../../include/FLAC/format.h:176: error: previous declaration of
'FLAC__STREAM_SYNC' was here
[many others follow]



Also MSVC fails because src/libFLAC/include/private/bitmath.h now
contains "uint32_t long idx" instead of "unsigned long idx".

According to MSDN _BitScanReverse*() functions have signatures:
unsigned char _BitScanReverse(unsigned long *, unsigned long);
unsigned char _BitScanReverse64(unsigned long *, unsigned __int64);

Other than that, it compiles everything without errors.
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[Ozkan Sezer]

On 1/14/17, Erik de Castro Lopo  wrote:
> Ozkan Sezer wrote:
>
>> > Ozkan Sezer wrote:
>> >
>> >> unsigned int and FLAC__uint32 are used interchangeably, leading to
>> >> warnings with platforms (e.g. djgpp) where int32_t is long:
>> >
>> > Is `sizeof int == 4` though?
>>
>> Yes, obviously it is
>
> Well I've just pushed a patch that purges the code base of `unsigned`.
> Please test and let us know how it goes.

Well, the commit seems like overkill :)  not all the unsigned needed
converting.

And build fails :
format.c:53: error: conflicting types for 'FLAC__STREAM_SYNC'
../../include/FLAC/format.h:176: error: previous declaration of
'FLAC__STREAM_SYNC' was here
[many others follow]
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[Erik de Castro Lopo]

Ozkan Sezer wrote:

> > Ozkan Sezer wrote:
> >
> >> unsigned int and FLAC__uint32 are used interchangeably, leading to
> >> warnings with platforms (e.g. djgpp) where int32_t is long:
> >
> > Is `sizeof int == 4` though?
> 
> Yes, obviously it is

Well I've just pushed a patch that purges the code base of `unsigned`.
Please test and let us know how it goes.

Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[Ozkan Sezer]

On 1/14/17, Erik de Castro Lopo  wrote:
> Ozkan Sezer wrote:
>
>> unsigned int and FLAC__uint32 are used interchangeably, leading to
>> warnings with platforms (e.g. djgpp) where int32_t is long:
>
> Is `sizeof int == 4` though?

Yes, obviously it is

>  I couldn't image FLAC working correctly
> if it wasn't.
>
> Erik
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] unsigned int and FLAC__uint32 are used interchangeably

[Erik de Castro Lopo]

Ozkan Sezer wrote:

> unsigned int and FLAC__uint32 are used interchangeably, leading to
> warnings with platforms (e.g. djgpp) where int32_t is long:

Is `sizeof int == 4` though? I couldn't image FLAC working correctly
if it wasn't.

Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev