flac dev - Apr 2014 - Patch to fix compiler warnings and error status collisions

Hi,

I am new around here, so I am not sure of all your procedures for submitting
patches/pull requests.

The attached patch fixes all Clang compilation warnings.

Note, some of these warnings were real problems. There is the potential for API
users to misinterpret the state being returned since the return variable was of
type FLAC__StreamDecoderState, but the value being returned was
FLAC__StreamDecoderInitStatus. When
FLAC__STREAM_DECODER_INIT_STATUS_ALREADY_INITIALIZED was returned in this case
it was indistinguishable from FLAC__STREAM_DECODER_OGG_ERROR.

Is there somewhere that I can push the patch so that it can easily be pulled
into the repository or does one of the maintainers just apply the patch and
attribute the credit in the commit message?

Thanks,
-Lenny
-------------- next part --------------
A non-text attachment was scrubbed...
Name: flac-warnings.diff
Type: application/octet-stream
Size: 4982 bytes
Desc: not available
Url :
http://lists.xiph.org/pipermail/flac-dev/attachments/20140406/922ad2b4/attachment.obj

Lenny Maiorani wrote:
> I am new around here, so I am not sure of all your procedures
> for submitting patches/pull requests.
A patch like you sent is fine. When I commit that I will add
a Patch-from line to the commit.

Otherwise, you can commit to you local repo and then use the git
format-patch sub command to create a set or patches that can
be attached to an email and sent to the list.
> The attached patch fixes all Clang compilation warnings.
I'll check this out.
 
> Note, some of these warnings were real problems. There is the
> potential for API users to misinterpret the state being returned
> since the return variable was of type FLAC__StreamDecoderState,
> but the value being returned was FLAC__StreamDecoderInitStatus.
> When FLAC__STREAM_DECODER_INIT_STATUS_ALREADY_INITIALIZED was
> returned in this case it was indistinguishable from
> FLAC__STREAM_DECODER_OGG_ERROR.
Problem is this patch changes the ABI because FLAC__StreamDecodeInitStatus
disappears from the API.

Is there some way this can be fixed without chaning the API/ABI?
 
Cheers,
Erik
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/

Lenny Maiorani wrote:
> Hi,
>
> I am new around here, so I am not sure of all your procedures for
submitting patches/pull requests.
>
> The attached patch fixes all Clang compilation warnings.
BTW, there was a discussion about FLAC__CPUINFO_IA32_CPUID_ZZZZ constants:

http://lists.xiph.org/pipermail/flac-dev/2014-March/004623.html and next
messages

On Apr 7, 2014, at 4:07 AM, Erik de Castro Lopo <mle+la at mega-nerd.com>
wrote:
> Lenny Maiorani wrote:
> 
>> I am new around here, so I am not sure of all your procedures
>> for submitting patches/pull requests.
> 
> A patch like you sent is fine. When I commit that I will add
> a Patch-from line to the commit.
> 
> Otherwise, you can commit to you local repo and then use the git
> format-patch sub command to create a set or patches that can
> be attached to an email and sent to the list.
> 
>> The attached patch fixes all Clang compilation warnings.
> 
> I'll check this out.
> 
>> Note, some of these warnings were real problems. There is the
>> potential for API users to misinterpret the state being returned
>> since the return variable was of type FLAC__StreamDecoderState,
>> but the value being returned was FLAC__StreamDecoderInitStatus.
>> When FLAC__STREAM_DECODER_INIT_STATUS_ALREADY_INITIALIZED was
>> returned in this case it was indistinguishable from
>> FLAC__STREAM_DECODER_OGG_ERROR.
> 
> Problem is this patch changes the ABI because FLAC__StreamDecodeInitStatus
> disappears from the API.
> 
> Is there some way this can be fixed without chaning the API/ABI?
FLAC__StreamDecodeInitStatus is still there. It is simply another name for
FLAC__StreamDecoderState. The API does not change here, other than the numerical
value of the enums. However, the bug that exists is that the numerical value of
the enums collide and are being used interchangeably in
decoder->protected_->state, so they really should be the same enum, IMHO.

-Lenny

> 
> Cheers,
> Erik
> -- 
> ----------------------------------------------------------------------
> Erik de Castro Lopo
> http://www.mega-nerd.com/
> _______________________________________________
> flac-dev mailing list
> flac-dev at xiph.org
> http://lists.xiph.org/mailman/listinfo/flac-dev