Re: [PATCH] Setter methods for names/types of a struct type

[Jose E. Marchesi]

[And re-replying on my side :)]

    Hello, I am very glad we get to continue our project :).
    
    > I can't see why we would want to provide these services in libpoke...
    
    In order to build structs/arrays we need to provide the 3rd argument to
    the corresponding functions which is an struct/array type.
    Therefore, we need an interface to build a struct/array type.
    
    Basically, I needed these functions to build structs/arrays by hand in
    order to test the functionality of pk-mi-json-to-val and
    pk-mi-val-to-json.
    
    I am not very sure if we truly need them.

I was referring to pk_alloc and pk_assert, not to the type builders.  I
don't think we want to expose these two calls to the callers of libpoke.

As for testing, I am working today on unit test infrastructure.
    
    > The patch is missing the changes in pk-val.c.
    > Can you please re-submit?
    > Thanks!
    
    Yes, sorry for that.

You forgot to CC the list :)
    
    diff --git a/libpoke/libpoke.h b/libpoke/libpoke.h
    index bd829ff5..0574228a 100644
    --- a/libpoke/libpoke.h
    +++ b/libpoke/libpoke.h
    
    +void *pk_alloc (size_t size);
    +
    +void pk_assert (int expression);
    +
     /* Signed integers.  */
    
     /* Build and return a poke integer.
    @@ -490,10 +494,11 @@ pk_val pk_offset_unit (pk_val val);
     /* Structs. */
    
     /* Build and return a poke struct.
    +
        NFIELDS is an uint<64> PK value specifying the number of fields
        in the struct.  This can be uint<64>0 for an empty struct.
    
    -   TYPE is a type PK value specifying the type of the array.
    +   TYPE is a type PK value specifying the type of the struct.
    
        The fields and methods in the created struct are initialized to
        PK_NULL.*/
    @@ -518,7 +523,9 @@ pk_val pk_struct_field_boffset (pk_val sct, uint64_t
    idx);
        beginning of the struct.
    
        ARRAY is the struct value.
    +
        IDX is the index of the field in the struct.
    +
        BOFFSET is an uint<64> value with the bit-offset of the field.  */
    

Hm, there is something wrong with the formatting of the patches.  Is
your MUA splitting long lines or something?  To avoid that, you may want
to send the patches attached as text/x-patch or text/x-diff.

     void pk_struct_set_field_boffset (pk_val sct, uint64_t idx, pk_val offset);
    @@ -526,6 +533,7 @@ void pk_struct_set_field_boffset (pk_val sct, uint64_t
    idx, pk_val offset);
     /* Get the NAME of the struct field.
    
        SCT is the struct value.
    +
        IDX is the index of the field in the struct. */
    
     pk_val pk_struct_field_name (pk_val sct, uint64_t idx);
    @@ -533,7 +541,9 @@ pk_val pk_struct_field_name (pk_val sct, uint64_t idx);
     /* Set the NAME of the struct field.
    
        SCT is the struct value.
    +
        IDX is the index of the field in the struct.
    +
        NAME is the string name for this field. */
    
     void pk_struct_set_field_name (pk_val sct, uint64_t idx, pk_val name);
    @@ -541,6 +551,7 @@ void pk_struct_set_field_name (pk_val sct, uint64_t
    idx, pk_val name);
     /* Get the VALUE of the struct field.
    
        SCT is the struct value.
    +
        IDX is the index of the field in the struct. */
    
     pk_val pk_struct_field_value (pk_val sct, uint64_t idx);
    @@ -548,13 +559,13 @@ pk_val pk_struct_field_value (pk_val sct, uint64_t
    idx);
     /* Set the VALUE of the struct field.
    
        SCT is the struct value.
    +
        IDX is the index of the field in the struct.
    +
        VALUE is the new value for this field. */
    
     void pk_struct_set_field_value (pk_val sct, uint64_t idx, pk_val value);
    
    -
    -
     /* Arrays.  */
    
     /* Build and return a poke array.
    @@ -674,27 +685,53 @@ pk_val pk_offset_type_unit (pk_val type);
    
     pk_val pk_make_struct_type (pk_val nfields, pk_val name, pk_val *fnames,
    pk_val *ftypes);
    
    +/* Get the struct type.  */
    +
    +pk_val pk_struct_type (pk_val sct);
    +
     /* Get the name of the struct type.  */
    
    -pk_val pk_struct_type_name (pk_val type);
    +pk_val pk_struct_type_name (pk_val sct_type);
    

This is a matter of personal preference, but `type' is perfectly ok for
the main type being referred in the call.  In this case, the struc type.

When two types are passed as arguments, like in
pk_struct_type_set_ftype, the main subject type can still be `type' and
the other `field_type' or something like that.


     /* Get the number of fields of the struct type.  */
    
    -pk_val pk_struct_type_nfields (pk_val type);
    +pk_val pk_struct_type_nfields (pk_val sct_type);
    
     /* Get the name of a field in the struct type.
    
        TYPE is the struct type.
    -   IDX is the index of the struct field. */
    +
    +   IDX is the index of the field in the struct type. */
    +
    +pk_val pk_struct_type_fname (pk_val sct_type, uint64_t idx);
    +
    +/* Set the name of a field the struct type.
    +
    +   TYPE is the struct type.
    +
    +   IDX is the index of the field in the struct type.
    +
    +   NAME is a string containing the name of the field in the struct type. */
    
    -pk_val pk_struct_type_fname (pk_val type, uint64_t idx);
    +void pk_struct_type_set_fname (pk_val sct_type, uint64_t idx, pk_val name);
    
     /* Get type of a field in the struct.
    
        TYPE is the struct type.
    +
        IDX is the index of the struct field.  */
    
    -pk_val pk_struct_type_ftype (pk_val type, uint64_t idx);
    +pk_val pk_struct_type_ftype (pk_val sct_type, uint64_t idx);
    +
    +/* Set the type of a field the struct type.
    +
    +   TYPE is the struct type.
    +
    +   IDX is the index of the field in the struct type.
    +
    +   TYPE is the type of the field in the struct type. */
    +
    +void pk_struct_type_set_ftype (pk_val sct_type, uint64_t idx, pk_val type);
    
    diff --git a/libpoke/pk-val.c b/libpoke/pk-val.c
    index b1fcc85e..12fc7593 100644
    --- a/libpoke/pk-val.c
    +++ b/libpoke/pk-val.c
    
    +void*
    +pk_alloc (size_t size)
    +{
    +  return pvm_alloc (size);
    +}
    +
    +void
    +pk_assert (int expression)
    +{
    +  return pvm_assert (expression);
    +}
    +
    
     void pk_struct_set_field_value (pk_val sct, uint64_t idx, pk_val value)
     {
    -  /*NOTE to jemarch: please tell me if you dont agree with this 
pvm_assert*/
    -  pk_val sct_type = PVM_VAL_SCT_TYPE (sct);
    -  pvm_assert(pk_type_code(pk_typeof(value)) ==
    pk_type_code(PVM_VAL_TYP_S_FTYPE(sct_type, idx)));
       PVM_VAL_SCT_FIELD_VALUE (sct, idx) = value;
     }
    
    @@ -311,27 +316,45 @@ pk_make_struct_type (pk_val nfields, pk_val name,
    pk_val *fnames, pk_val *ftypes
     }
    
     pk_val
    -pk_struct_type_name (pk_val type)
    +pk_struct_type (pk_val sct)
    +{
    +  return PVM_VAL_SCT_TYPE (sct);
    +}
    +
    +pk_val
    +pk_struct_type_name (pk_val sct_type)
     {
    -  return PVM_VAL_TYP_S_NAME (type);
    +  return PVM_VAL_TYP_S_NAME (sct_type);
     }
    
     pk_val
    -pk_struct_type_nfields (pk_val type)
    +pk_struct_type_nfields (pk_val sct_type)
     {
    -  return PVM_VAL_TYP_S_NFIELDS (type);
    +  return PVM_VAL_TYP_S_NFIELDS (sct_type);
     }
    
     pk_val
    -pk_struct_type_fname (pk_val type, uint64_t idx)
    +pk_struct_type_fname (pk_val sct_type, uint64_t idx)
    +{
    +  return PVM_VAL_TYP_S_FNAME (sct_type, idx);
    +}
    +
    +void
    +pk_struct_type_set_fname (pk_val sct_type, uint64_t idx, pk_val name)
     {
    -  return PVM_VAL_TYP_S_FNAME (type, idx);
    +  PVM_VAL_TYP_S_FNAME (sct_type, idx) = name;
     }
    
     pk_val
    -pk_struct_type_ftype (pk_val type, uint64_t idx)
    +pk_struct_type_ftype (pk_val sct_type, uint64_t idx)
    +{
    +  return PVM_VAL_TYP_S_FTYPE (sct_type, idx);
    +}
    +
    +void
    +pk_struct_type_set_ftype (pk_val sct_type, uint64_t idx, pk_val type)
     {
    -  return PVM_VAL_TYP_S_FTYPE (type, idx);
    +  PVM_VAL_TYP_S_FTYPE (sct_type, idx) = type;
     }