> > > There's a problem with the trunk. After performing 'make' I get: > > > > > > pdf-filter.c: In function 'install_filters': > > > pdf-filter.c:1317: error: 'JBIG2DEC_GLOBAL_SEGMENTS_ARG' undeclared > > > (first use in this function) > > > pdf-filter.c:1317: error: (Each undeclared identifier is reported only > > > once > > > pdf-filter.c:1317: error: for each function it appears in.) > > > pdf-filter.c:1317: error: 'JBIG2DEC_PAGE_SIZE' undeclared (first use in > > > this function) > > > > > > It seems that macro IS_FILTER_ARG compares with JBIG2DEC even when it's > > > not declared. I send a patch. > > > > Yes, I can confirm this problem (if no HAVE_LIBJBIG2DEC). Your patch is > > ok, but we would need an extra macro for each further configuration > > option with arguments (and combinations). Are any further configuration > > option with arguments planed? > > > > Maybe it is better to declare all values inside the enum (before the > > macro), or to make a function instead of the macro (so #ifdef can be > > used inside). But this can be done later too, so decide for yourself. > > > > Regards, > > Georg > > > > -----BEGIN PGP SIGNATURE----- > > Version: GnuPG v2.0.17 (GNU/Linux) > > Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ > > > > iEYEARECAAYFAk6GSDUACgkQ5sLITM1qIaKpPgCfWCDHzE8ueUyBpddgtzrXRQKd > > 4eEAnR9Itm/A2CTDbQv0KxZa2rn687t+ > > =xAIr > > -----END PGP SIGNATURE----- > > Yes, in case others were planned it would become a mess. So I settle for > the function idea. Here it goes. >
Please attach always uncompressed files, with .diff or .txt extensions;
it is much easier to review the patches inline.
Just some coding style comments...
> +static pdf_bool_t
> +is_filter_arg(enum filter_arg arg)
Whitespace needed before the parenthesis.
> + return (arg) == PRED_COLORS_ARG
> + || (arg) == PRED_BITSPERCOMPONENT_ARG
> + || (arg) == PRED_COLUMNS_ARG
> + || (arg) == PRED_PREDICTOR_ARG
I don't usually split the lines with multiple conditions before the
operator, but the GCS suggests to do it. I would add an extra
parenthesis to group all conditions, remove the parenthesis around
'arg' (not a macro anymore), and get all conditions aligned, like:
return (arg == PRED_COLORS_ARG
|| arg == PRED_BITSPERCOMPONENT_ARG
|| arg == PRED_COLUMNS_ARG
|| arg == PRED_PREDICTOR_ARG
...);
> + if (!is_filter_arg(next_ci) && filter_to_install !=
> FILTER_INSTALL_NONE)
whitespace needed before parenthesis.
Cheers!
--
Aleksander
signature.asc
Description: This is a digitally signed message part
