Skip to content

Conversation

@rkuchumov
Copy link
Member

So, I decided to remove warnings. I'll test on correct files and write whether it's OK or not.

@cfsmp3
Copy link
Contributor

cfsmp3 commented Jul 3, 2014

Don't know about some of these changes. For example:

        read_u32(esstream); // Advance bitstream
        (void) read_u32(esstream); // Advance bitstream

I think that's a "fix" to remove a warning from a braindead compiler. Is it
really complaining about the return value being discarded? If yes, I think
it's best to create skip_u32() and use it, rather than add (void)
everywhere in which we don't care about the return value.

In other places the variables are not being used because they are for
debug, but the debug lines are still there (just commented out), for
example:

            int32_t dwVersion = 0;
            int32_t unknown = 0;

I think it's better to also comment out the variable definitions than
remove everything (variables and the commented out code that used them).
After all, we know that from time to time we need to debug things again...
a sample that doesn't work, a segfault, etc, and we'd need to add the debug
code again.

In init_avc()

else
{

/* CFS: Warning!!: Untested stuff, copied from specs (8.2.1.3) */
LLONG FrameNumOffset = 0;

That code should be left there. Comment out, OK. Completely remove no. I
added it when debugging some thing and I typed the spec completely
(couldn't test). I'm sure eventually we'll find the right sample.

In xds.c, I try to parse all the fields. Some of the fields are not
displayed later to the user (which is why they may cause the compiler to
complain), but the are useful when debugging XDS. So we cannot completely
remove those.

On Thu, Jul 3, 2014 at 2:49 PM, Ruslan notifications@github.com wrote:

So, I decided to remove warnings. I'll test on correct files and write

whether it's OK or not.

You can merge this Pull Request by running

git pull https://github.com/rkuchumov/ccextractor cleanup

Or view, comment on, or merge it at:

#62
Commit Summary

  • removed warnings (-Wall)

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#62.

@rkuchumov
Copy link
Member Author

OK, I'll comment instead of deleting and add skip_u32().

@rkuchumov
Copy link
Member Author

So, I commented instead of deleleting and removed a few variables which are just declared but not metioned anywhere. Also tested on correct files, everything seems ok.

@cfsmp3 cfsmp3 merged commit 134762d into CCExtractor:master Jul 4, 2014
@rkuchumov rkuchumov deleted the cleanup branch July 28, 2014 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants