Re: Partial fix for CET 12-14a, 12-14b 12-14e, 12-14f, 12-14g

Alex,

I approve the patch.
Could you please also replace 'foo' with a better identifier ?
Please commit with that.

Igor.



----- Original Message ----- 
From: "Alex Cherepanov" <[email protected]>
To: "gs-code-review" <[email protected]>
Sent: Monday, October 30, 2006 7:24 AM
Subject: [gs-code-review] Partial fix for CET 12-14a, 12-14b 12-14e, 
12-14f,12-14g


> This is a new version of my previous patch that covers more test
> cases and relocates some code.
>
> Fix decoding of the indexed color space in the smooth shading.
> Adjust error codes to satisfy CET 12-14a, 12-14b 12-14e, 12-14f, 12-14g
> Change gs_errorinfo_put_pair() to take char* instead of byte* a
> more common type and move all errorinfo-related functions to idparam.c
> because they are most often used with dictionary parameter functions.
>
> DIFFERENCES:
> Testing now
>
>
>


--------------------------------------------------------------------------------


> Index: gs/src/int.mak
> ===================================================================
> --- gs/src/int.mak (revision 7132)
> +++ gs/src/int.mak (working copy)
> @@ -174,8 +174,8 @@
>  $(PSCC) $(PSO_)idict.$(OBJ) $(C_) $(PSSRC)idict.c
>
> $(PSOBJ)idparam.$(OBJ) : $(PSSRC)idparam.c $(GH) $(memory__h) $(string__h) 
> $(ierrors_h)\
> - $(gsmatrix_h) $(gsuid_h)\
> - $(idict_h) $(idparam_h) $(ilevel_h) $(imemory_h) $(iname_h) $(iutil_h)\
> + $(gsmatrix_h) $(gsuid_h) $(dstack_h)\
> + $(idict_h) $(iddict_h) $(idparam_h) $(ilevel_h) $(imemory_h) $(iname_h) 
> $(iutil_h)\
>  $(oper_h) $(store_h)
>  $(PSCC) $(PSO_)idparam.$(OBJ) $(C_) $(PSSRC)idparam.c
>
> @@ -478,7 +478,7 @@
>
> $(PSOBJ)zimage.$(OBJ) : $(PSSRC)zimage.c $(OP) $(memory__h)\
>  $(gscspace_h) $(gscssub_h) $(gsimage_h) $(gsmatrix_h) $(gsstruct_h)\
> - $(gxiparam_h) $(interp_h)\
> + $(gxiparam_h)\
>  $(estack_h) $(ialloc_h) $(ifilter_h) $(igstate_h) $(iimage_h) 
> $(ilevel_h)\
>  $(store_h) $(stream_h)
>  $(PSCC) $(PSO_)zimage.$(OBJ) $(C_) $(PSSRC)zimage.c
> Index: gs/src/zshade.c
> ===================================================================
> --- gs/src/zshade.c (revision 7132)
> +++ gs/src/zshade.c (working copy)
> @@ -165,8 +165,10 @@
>  int num_comp = gs_color_space_num_components(pcs_orig);
>  gs_color_space *pcs;
>
> - if (num_comp < 0) /* Pattern color space */
> -     return_error(e_rangecheck);
> + if (num_comp < 0) { /* Pattern color space */
> +            gs_errorinfo_put_pair_from_dict(i_ctx_p, op, "ColorSpace");
> +     return_error(e_typecheck);
> +        }
>  pcs = ialloc_struct(gs_color_space, &st_color_space,
>      "build_shading");
>  if (pcs == 0)
> @@ -211,11 +213,15 @@
>      params.BBox.q.y = box[1];
>         }
>  params.have_BBox = true;
> -    } else
> +    } else {
> +        gs_errorinfo_put_pair_from_dict(i_ctx_p, op, "BBox");
>  goto fail;
> +    }
>     code = dict_bool_param(op, "AntiAlias", false, &params.AntiAlias);
> -    if (code < 0)
> +    if (code < 0) {
> +        gs_errorinfo_put_pair_from_dict(i_ctx_p, op, "AntiAlias");
>  goto fail;
> +    }
>     /* Finish building the shading. */
>     code = (*proc)(i_ctx_p, op, &params, &psh, imemory);
>     if (code < 0)
> @@ -289,9 +295,15 @@
>  * Adobe interpreters follow PLRM in this respect and we follow them.
>  */
> private int
> -check_indexed_vs_function(const gs_color_space *pcs, const gs_function_t 
> *foo)
> -{ if (foo && gs_color_space_get_index(pcs) == 
> gs_color_space_index_Indexed)
> -    return_error(e_rangecheck);
> +check_indexed_vs_function(i_ctx_t *i_ctx_p, const ref *op,
> +                          const gs_color_space *pcs, const gs_function_t 
> *foo)
> +{ if (foo && gs_color_space_get_index(pcs) == 
> gs_color_space_index_Indexed) {
> +    static const char fn[] = "Function";
> +    ref *f;
> +    if (dict_find_string(op, fn, &f) > 0)
> +        gs_errorinfo_put_pair(i_ctx_p, fn, sizeof(fn) - 1, f);
> +    return_error(e_typecheck);  /* CET 12-14a */
> +  }
>   return 0;
> }
>
> @@ -310,21 +322,40 @@
>     *(gs_shading_params_t *)&params = *pcommon;
>     gs_make_identity(&params.Matrix);
>     params.Function = 0;
> -    if ((code = dict_floats_param(imemory, op, "Domain",
> -   4, params.Domain,
> -   default_Domain)) < 0 ||
> - (dict_find_string(op, "Matrix", &pmatrix) > 0 &&
> - (code = read_matrix(imemory, pmatrix, &params.Matrix)) < 0) ||
> - (code = build_shading_function(i_ctx_p, op, &params.Function, 2, mem)) < 
> 0 ||
> - (code = check_indexed_vs_function(params.ColorSpace, params.Function)) < 
> 0 ||
> - (code = gs_shading_Fb_init(ppsh, &params, mem)) < 0
> - ) {
> +    code = dict_floats_param_errorinfo(i_ctx_p, op, "Domain",
> +   4, params.Domain, default_Domain);
> +    if (code < 0)
> +        goto out;
> +    if (params.Domain[0] > params.Domain[2] || params.Domain[1] > 
> params.Domain[3]) {
> +        gs_errorinfo_put_pair_from_dict(i_ctx_p, op, "Domain");
> +        code = gs_note_error(e_rangecheck);
> +        goto out; /* CPSI 3017 and CET 12-14b reject un-normalised box */
> +    }
> +    if (dict_find_string(op, "Matrix", &pmatrix) > 0 ) {
> +        code = read_matrix(imemory, pmatrix, &params.Matrix);
> +        if (code < 0) {
> +            gs_errorinfo_put_pair_from_dict(i_ctx_p, op, "Matrix");
> +            goto out;
> +        }
> +    }
> +    code = build_shading_function(i_ctx_p, op, &params.Function, 2, mem);
> +    if (code < 0) {
> + gs_errorinfo_put_pair_from_dict(i_ctx_p, op, "Function");
> +        goto out;
> +    }
> +    if (params.Function == 0) {  /* Function is required */
> + code = gs_note_error(e_undefined);
> + gs_errorinfo_put_pair_from_dict(i_ctx_p, op, "Function");
> +        goto out;
> +    }
> +    code = check_indexed_vs_function(i_ctx_p, op, params.ColorSpace, 
> params.Function);
> +    if (code < 0)
> +        goto out;
> +    code = gs_shading_Fb_init(ppsh, &params, mem);
> + out:;
> +    if (code < 0 && params.Function)
>  gs_free_object(mem, params.Function, "Function");
> - return code;
> -    }
> -    if (params.Function == 0) /* Function is required */
> - return_error(e_undefined);
> -    return 0;
> +    return code;
> }
> /* <dict> .buildshading1 <shading_struct> */
> private int
> @@ -346,7 +377,7 @@
>
>     *pFunction = 0;
>     if (code < 0 ||
> - (code = dict_floats_param(imemory, op, "Domain", 2, Domain,
> + (code = dict_floats_param_errorinfo(i_ctx_p, op, "Domain", 2, Domain,
>    default_Domain)) < 0 ||
>  (code = build_shading_function(i_ctx_p, op, pFunction, 1, mem)) < 0
>  )
> @@ -385,7 +416,7 @@
>     if ((code = build_directional_shading(i_ctx_p, op, params.Coords, 4,
>    params.Domain, &params.Function,
>    params.Extend, mem)) < 0 ||
> - (code = check_indexed_vs_function(params.ColorSpace, params.Function)) < 
> 0 ||
> + (code = check_indexed_vs_function(i_ctx_p, op, params.ColorSpace, 
> params.Function)) < 0 ||
>  (code = gs_shading_A_init(ppsh, &params, mem)) < 0
>  ) {
>  gs_free_object(mem, params.Function, "Function");
> @@ -411,7 +442,7 @@
>     if ((code = build_directional_shading(i_ctx_p, op, params.Coords, 6,
>    params.Domain, &params.Function,
>    params.Extend, mem)) < 0 ||
> - (code = check_indexed_vs_function(params.ColorSpace, params.Function)) < 
> 0 ||
> + (code = check_indexed_vs_function(i_ctx_p, op, params.ColorSpace, 
> params.Function)) < 0 ||
>  (code = gs_shading_R_init(ppsh, &params, mem)) < 0
>  ) {
>  gs_free_object(mem, params.Function, "Function");
> @@ -540,7 +571,7 @@
>     if ((code =
>  build_mesh_shading(i_ctx_p, op, (gs_shading_mesh_params_t *)&params,
>      &params.Decode, &params.Function, mem)) < 0 ||
> - (code = check_indexed_vs_function(params.ColorSpace, params.Function)) < 
> 0 ||
> + (code = check_indexed_vs_function(i_ctx_p, op, params.ColorSpace, 
> params.Function)) < 0 ||
>  (code = flag_bits_param(op, (gs_shading_mesh_params_t *)&params,
>  &params.BitsPerFlag)) < 0 ||
>  (code = gs_shading_FfGt_init(ppsh, &params, mem)) < 0
> @@ -569,7 +600,7 @@
>     if ((code =
>  build_mesh_shading(i_ctx_p, op, (gs_shading_mesh_params_t *)&params,
>      &params.Decode, &params.Function, mem)) < 0 ||
> - (code = check_indexed_vs_function(params.ColorSpace, params.Function)) < 
> 0 ||
> + (code = check_indexed_vs_function(i_ctx_p, op, params.ColorSpace, 
> params.Function)) < 0 ||
>  (code = dict_int_param(op, "VerticesPerRow", 2, max_int, 0,
>         &params.VerticesPerRow)) < 0 ||
>  (code = gs_shading_LfGt_init(ppsh, &params, mem)) < 0
> @@ -598,7 +629,7 @@
>     if ((code =
>  build_mesh_shading(i_ctx_p, op, (gs_shading_mesh_params_t *)&params,
>      &params.Decode, &params.Function, mem)) < 0 ||
> - (code = check_indexed_vs_function(params.ColorSpace, params.Function)) < 
> 0 ||
> + (code = check_indexed_vs_function(i_ctx_p, op, params.ColorSpace, 
> params.Function)) < 0 ||
>  (code = flag_bits_param(op, (gs_shading_mesh_params_t *)&params,
>  &params.BitsPerFlag)) < 0 ||
>  (code = gs_shading_Cp_init(ppsh, &params, mem)) < 0
> @@ -627,7 +658,7 @@
>     if ((code =
>  build_mesh_shading(i_ctx_p, op, (gs_shading_mesh_params_t *)&params,
>      &params.Decode, &params.Function, mem)) < 0 ||
> - (code = check_indexed_vs_function(params.ColorSpace, params.Function)) < 
> 0 ||
> + (code = check_indexed_vs_function(i_ctx_p, op, params.ColorSpace, 
> params.Function)) < 0 ||
>  (code = flag_bits_param(op, (gs_shading_mesh_params_t *)&params,
>  &params.BitsPerFlag)) < 0 ||
>  (code = gs_shading_Tpp_init(ppsh, &params, mem)) < 0
> Index: gs/src/zimage.c
> ===================================================================
> --- gs/src/zimage.c (revision 7132)
> +++ gs/src/zimage.c (working copy)
> @@ -27,7 +27,6 @@
> #include "gxiparam.h"
> #include "idict.h"
> #include "idparam.h"
> -#include "interp.h"             /* for gs_errorinfo_put_pair() */
> #include "estack.h" /* for image[mask] */
> #include "ialloc.h"
> #include "igstate.h"
> @@ -298,7 +297,7 @@
>  break;
>      default:
>  if (!r_is_proc(sources)) {
> -        static const byte ds[] = "DataSource";
> +        static const char ds[] = "DataSource";
>                     if (pie != NULL)
>                         gx_image_end(pie, false);    /* Clean up pie */
>                     gs_errorinfo_put_pair(i_ctx_p, ds, sizeof(ds) - 1, 
> pp);
> Index: gs/src/idparam.c
> ===================================================================
> --- gs/src/idparam.c (revision 7132)
> +++ gs/src/idparam.c (working copy)
> @@ -19,7 +19,9 @@
> #include "ierrors.h"
> #include "gsmatrix.h" /* for dict_matrix_param */
> #include "gsuid.h"
> +#include "dstack.h"             /* for systemdict */
> #include "idict.h"
> +#include "iddict.h"
> #include "idparam.h" /* interface definition */
> #include "ilevel.h"
> #include "imemory.h" /* for iutil.h */
> @@ -269,6 +271,26 @@
>  e_rangecheck, e_rangecheck);
> }
>
> +
> +/* Do dict_floats_param() and store [/key any] array in $error.errorinfo
> + * on failure. The key must be a permanently allocated C string.
> + */
> +int
> +dict_floats_param_errorinfo(i_ctx_t *i_ctx_p,
> +   const ref * pdict, const char *kstr,
> +   uint maxlen, float *fvec, const float *defaultvec)
> +{
> +    ref *val;
> +    int code = dict_float_array_check_param(imemory, pdict, kstr, maxlen,
> +                       fvec, defaultvec, e_rangecheck, e_rangecheck);
> +    if (code < 0) {
> +       if (dict_find_string(pdict, kstr, &val) > 0)
> +          gs_errorinfo_put_pair(i_ctx_p, kstr, strlen(kstr), val);
> +    }
> +    return code;
> +}
> +
> +
> /*
>  * Get a procedure from a dictionary.  If the key is missing,
>  *      defaultval = false means substitute t__invalid;
> @@ -403,3 +425,44 @@
>  puniqueid->value.intval == puid->id);
>     }
> }
> +
> +/* Create and store [/key any] array in $error.errorinfo.
> + * The key must be a permanently allocated C string.
> + * This routine is here because it is often used with parameter 
> dictionaries.
> + */
> +int
> +gs_errorinfo_put_pair(i_ctx_t *i_ctx_p, const char *key, int len, const 
> ref *any)
> +{
> +    int code;
> +    ref pair, *aptr, key_name, *pderror;
> +
> +    code = name_ref(imemory_local, (const byte *)key, len, &key_name, 0);
> +    if (code < 0)
> +        return code;
> +    code = gs_alloc_ref_array(iimemory_local, &pair, a_readonly, 2, 
> "gs_errorinfo_put_pair");
> +    if (code < 0)
> +        return code;
> +    aptr = pair.value.refs;
> +    ref_assign_new(aptr, &key_name);
> +    ref_assign_new(aptr+1, any);
> +    if (dict_find_string(systemdict, "$error", &pderror) <= 0 ||
> + !r_has_type(pderror, t_dictionary) ||
> + idict_put_string(pderror, "errorinfo", &pair) < 0
> + )
> + return_error(e_Fatal);
> +    return 0;
> +}
> +
> +/* Take a key's value from a given dictionary, create [/key any] array,
> + * and store it in $error.errorinfo.
> + * The key must be a permanently allocated C string.
> + */
> +void
> +gs_errorinfo_put_pair_from_dict(i_ctx_t *i_ctx_p, const ref *op, const 
> char *key)
> +{   ref *val, n;
> +    if (dict_find_string(op, key, &val) <= 0) {
> +        make_null(&n);
> +        val = &n;
> +    }
> +    gs_errorinfo_put_pair(i_ctx_p, key, strlen(key), val);
> +}
> Index: gs/src/idparam.h
> ===================================================================
> --- gs/src/idparam.h (revision 7132)
> +++ gs/src/idparam.h (working copy)
> @@ -90,7 +90,15 @@
>        const ref * pdict, const char *kstr,
>        uint len, float *fvec,
>        const float *defaultvec);
> +/* Do dict_floats_param() and store [/key any] array in $error.errorinfo
> + * on failure. The key must be a permanently allocated C string.
> + */
> +int
> +dict_floats_param_errorinfo(i_ctx_t *i_ctx_p,
> +   const ref * pdict, const char *kstr,
> +   uint maxlen, float *fvec, const float *defaultvec);
>
> +
> /*
>  * For dict_proc_param,
>  *      defaultval = false means substitute t__invalid;
> @@ -108,4 +116,18 @@
> /* Check that a UID in a dictionary is equal to an existing, valid UID. */
> bool dict_check_uid_param(const ref * pdict, const gs_uid * puid);
>
> +
> +/* Create and store [/key any] array in $error.errorinfo.
> + * The key must be a permanently allocated C string.
> + */
> +int
> +gs_errorinfo_put_pair(i_ctx_t *i_ctx_p, const char *key, int len, const 
> ref *any);
> +
> +/* Take a key's value from a given dictionary, create [/key any] array,
> + * and store it in $error.errorinfo.
> + * The key must be a permanently allocated C string.
> + */
> +void
> +gs_errorinfo_put_pair_from_dict(i_ctx_t *i_ctx_p, const ref *op, const 
> char *key);
> +
> #endif /* idparam_INCLUDED */
> Index: gs/src/interp.c
> ===================================================================
> --- gs/src/interp.c (revision 7132)
> +++ gs/src/interp.c (working copy)
> @@ -725,33 +725,6 @@
>     return 0;
> }
>
> -/* Create and store [/key any] array in $error.errorinfo. */
> -/* The key must be a permanently allocated C string. */
> -/* This routine is here because of the proximity to the error handler. */
> -int
> -gs_errorinfo_put_pair(i_ctx_t *i_ctx_p, const byte *key, int len, const 
> ref *any)
> -{
> -    int code;
> -    ref pair, *aptr, key_name, *pderror;
> -
> -    code = name_ref(imemory_local, key, len, &key_name, 0);
> -    if (code < 0)
> -        return code;
> -    code = gs_alloc_ref_array(iimemory_local, &pair, a_readonly, 2, 
> "gs_errorinfo_put_pair");
> -    if (code < 0)
> -        return code;
> -    aptr = pair.value.refs;
> -    ref_assign_new(aptr, &key_name);
> -    ref_assign_new(aptr+1, any);
> -    if (dict_find_string(systemdict, "$error", &pderror) <= 0 ||
> - !r_has_type(pderror, t_dictionary) ||
> - idict_put_string(pderror, "errorinfo", &pair) < 0
> - )
> - return_error(e_Fatal);
> -    return 0;
> -}
> -
> -
> /* Main interpreter. */
> /* If execution terminates normally, return e_InterpreterExit. */
> /* If an error occurs, leave the current object in *perror_object */
> Index: gs/src/interp.h
> ===================================================================
> --- gs/src/interp.h (revision 7132)
> +++ gs/src/interp.h (working copy)
> @@ -61,10 +61,6 @@
> /* Put a string in $error /errorinfo. */
> int gs_errorinfo_put_string(i_ctx_t *, const char *);
>
> -/* Create and store [/key any] array in $error /errorinfo. */
> -/* The key must be a permanently allocated C string. */
> -int gs_errorinfo_put_pair(i_ctx_t *, const byte *key, int len, const ref 
> *any);
> -
> /* Initialize the interpreter. */
> int gs_interp_init(i_ctx_t **pi_ctx_p, const ref *psystem_dict,
>     gs_dual_memory_t *dmem);
> Index: gs/src/gxshade.c
> ===================================================================
> --- gs/src/gxshade.c (revision 7132)
> +++ gs/src/gxshade.c (working copy)
> @@ -241,16 +241,20 @@
>
>     if (index == gs_color_space_index_Indexed) {
>  int ncomp = gs_color_space_num_components(gs_cspace_base_space(pcs));
> - uint ci;
> - int code = cs->get_value(cs, num_bits, &ci);
> - gs_client_color cc;
> + int ci;
> +        float cf;
> + int code = cs->get_decoded(cs, num_bits, decode, &cf);
> +        gs_client_color cc;
>  int i;
>
>  if (code < 0)
>      return code;
> - if (ci >= gs_cspace_indexed_num_entries(pcs))
> + if (cf < 0)
>      return_error(gs_error_rangecheck);
> - code = gs_cspace_indexed_lookup(&pcs->params.indexed, (int)ci, &cc);
> + ci = (int)cf;
> +        if (ci >= gs_cspace_indexed_num_entries(pcs))
> +     return_error(gs_error_rangecheck);
> + code = gs_cspace_indexed_lookup(&pcs->params.indexed, ci, &cc);
>  if (code < 0)
>      return code;
>  for (i = 0; i < ncomp; ++i)
> Index: gs/lib/gs_ll3.ps
> ===================================================================
> --- gs/lib/gs_ll3.ps (revision 7132)
> +++ gs/lib/gs_ll3.ps (working copy)
> @@ -258,8 +258,19 @@
>   } if
>  % The .buildshading operators use the current color space
>  % for ColorSpace.
> -  dup /ShadingType get //.shadingtypes exch get
> -  1 index /ColorSpace get setcolorspace exec
> +  dup /ShadingType .knownget not { % error handling for CET 12-14b 
> conformance
> +     /$error .systemvar /errorinfo [ /ShadingType //null ] put
> +     /shfill .systemvar /undefined signalerror
> +  } if
> +  dup type /integertype ne {
> +     /$error .systemvar /errorinfo [ /ShadingType 4 index ] put
> +     /shfill .systemvar /typecheck signalerror
> +  } if
> +  //.shadingtypes 1 index .knownget not {
> +     /$error .systemvar /errorinfo [ /ShadingType 4 index ] put
> +     /shfill .systemvar /rangecheck signalerror
> +  } if
> +  exch pop 1 index /ColorSpace get setcolorspace exec
> } bind def
> systemdict /.reuseparamdict undef
>
>


--------------------------------------------------------------------------------


> _______________________________________________
> gs-code-review mailing list
> [email protected]
> http://www.ghostscript.com/mailman/listinfo/gs-code-review
>