Lenny Maiorani
2014-Apr-07 03:43 UTC
[flac-dev] 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
Erik de Castro Lopo
2014-Apr-07 10:07 UTC
[flac-dev] Patch to fix compiler warnings and error status collisions
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/
lvqcl
2014-Apr-07 10:53 UTC
[flac-dev] Patch to fix compiler warnings and error status collisions
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
Lenny Maiorani
2014-Apr-07 14:03 UTC
[flac-dev] Patch to fix compiler warnings and error status collisions
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
Possibly Parallel Threads
- Patch to fix compiler warnings and error status collisions
- Patch to fix compiler warnings and error status collisions
- Patch to fix compiler warnings and error status collisions
- Patch to fix compiler warnings and error status collisions
- [PATCH] Disable FLAC__bitreader_read_rice_signed_block_asm_ia32_bswap.