Skip to content
This repository has been archived by the owner on Jul 29, 2020. It is now read-only.

Lots of code cleanup #44

Merged
merged 50 commits into from
Jul 21, 2020
Merged

Lots of code cleanup #44

merged 50 commits into from
Jul 21, 2020

Conversation

MaxKellermann
Copy link
Contributor

All gcc/clang warnings are now fixed.

Commit 681296e did this only for clang, but GCC has the same
warning which we want to supress (until the code is fixed).
Errors from various called functions were stored in the `ret`
variable, but this variable was never checked.
This macro depends on the macro `max()` which is not provided by this library.
After commit 52cb87e, jpc_dec_dump() kept on using `PRIiFAST32`,
but in 32 bit mode, this should be `PRIiLEAST32`.  The new macro
`PRIjas_seqent` can be used instead.
Copy link
Contributor

@theta682 theta682 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this cleanup:

  1. Type safety
  2. constness
  3. Replacing old C-macros with inline functions
  4. Using unsigned types for countable values

@@ -972,6 +976,8 @@ static void render()
if (cmdopts.verbose) {
// fprintf(stderr, "vtlx=%f, vtly=%f, vsx=%f, vsy=%f\n",
// vtlx, vtly, gs.sx, gs.sy);
/* suppress -Wunused-but-set-variable */
(void)vtlx; (void)vtly;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend to split statements Declare one name (only) per declaration

Suggested change
(void)vtlx; (void)vtly;
(void)vtlx;
(void)vtly;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree on this one. It makes it more readable to me. But I don't care much and can merge anyways.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't supposed to be "readable". This is a kludge to work around a compiler warning for code which is commented out for unknown reasons. Usually, I wouldn't write two statements on one line, but in this case I decided to do so because I didn't want to waste any screen space for this cr*p.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether this is a good reason to break with the style ;)

src/libjasper/jpc/jpc_t1cod.c Show resolved Hide resolved
src/libjasper/jpc/jpc_t1enc.c Show resolved Hide resolved
@jubalh jubalh merged commit f12c24d into jasper-maint:master Jul 21, 2020
@MaxKellermann MaxKellermann deleted the cleanup3 branch July 29, 2020 11:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants