[Libvirt-cim] [PATCH] [CU][RFC](#2) Added lexer/parser support for string array properties

[Kaitlin Rupert]

Thanks Richard,

This looks really good.

> diff -r 1cb3975921d5 -r 0b79dd93e4ed eo_util_parser.y
> --- a/eo_util_parser.y	Mon Apr 27 16:11:32 2009 -0700
> +++ b/eo_util_parser.y	Sat May 30 14:03:04 2009 -0300

> @@ -20,15 +21,24 @@
>      /* specify prototypes to get rid of warnings */
>  int eo_parse_lex (void);
>  void eo_parse_error(char *);
> +inline void ins_chars_into_cmstr_arr(const CMPIBroker *broker,
> +                                     CMPIArray *arr,
> +                                     CMPICount index,
> +                                     char *str);
> 
>  #define RC_OK 0
>  #define RC_EOF EOF
>  #define RC_INVALID_CLASS -1000
> +#define EODEBUG 1

Be sure to remove this when you resubmit.

> 
>  /* DEFINE ANY GLOBAL VARS HERE */
>  static const CMPIBroker * _BROKER;
>  static CMPIInstance ** _INSTANCE;
>  static const char * _NAMESPACE;
> +static CMPICount stringarraysize;
> +static char **stringarray;
> +static char *stringarraypropname;
> +
> 

 > @@ -127,7 +138,17 @@
 >  			CMSetProperty(*_INSTANCE, $1, &($3), CMPI_boolean);
 >  			free($1);
 >  			}
 > +        |       PROPERTYNAME '=' OPENBRACKET
 > +                        {
 > +                        EOTRACE("propertyname = %s\n"
 > +				"\ttype = CMPI_charsA\n",
 > +				$1);

You assume the array will be a character array.  What about 
CMPI_uint16A, etc?

I think it's fine to not address integer types in this patch - handling 
arrays of string is complex enough.  Nut we should fix it in a follow up 
patch.

 >
 > +                        stringarraysize = 0;
 > +                        stringarraypropname = $1;
 > +                        }
 > +                arrayofstrings CLOSEBRACKET ';'
 > +
 >  	|	PROPERTYNAME '=' CIMNULL ';'
 >  			{
 >  			EOTRACE("propertyname = %s\n"

> @@ -136,11 +157,86 @@
>  			}
>  	;
> 
> +arrayofstrings: STRING 
> +                        {
> +                        EOTRACE("BootDevices[%u]=%s\n",stringarraysize, $1);

Make this trace statement more generic.

> +
> +                        stringarraysize++;
> +                        stringarray = (char **)realloc(stringarray, 
> +                                                       sizeof(char *) * 
> +                                                       stringarraysize);
> +                        stringarray[stringarraysize-1] = $1;
> +                        }
> +                COMMA arrayofstrings
> +
> +              
> +        |       STRING
> +                        {
> +                        CMPIArray *arr;
> +                        CMPICount i;
> +                        CMPIStatus s;
> +                        
> +                        EOTRACE("\tBootDevices[%u]=%s\n",stringarraysize, $1);

Same here.

> +                        
> +                        stringarraysize++;
> +
> +                        arr = CMNewArray(_BROKER, 
> +                                         stringarraysize, 
> +                                         CMPI_string, 
> +                                         &s);
> +                        if (s.rc != CMPI_RC_OK || CMIsNullObject(arr))
> +                                EOTRACE("Error creating array\n");

I would also return from here.  Otherwise, attempting to add values into 
the array will fail later on.

> +
> +                        // Values to stringarraysize - 2 are in the
> +                        // temporary array
> +                        for (i = 0; i < stringarraysize - 1; i++) {
> +                                ins_chars_into_cmstr_arr(_BROKER,
> +                                                         arr,
> +                                                         i,
> +                                                         stringarray[i]);

Trap any kind of error here and return.

> +
> +                                free(stringarray[i]);
> +                        }
> +                        
> +                        ins_chars_into_cmstr_arr(_BROKER, 
> +                                                 arr,  
> +                                                 stringarraysize - 1,
> +                                                 $1);

Same here.

> +
> +                        free($1);
> +
> +                        CMSetProperty(*_INSTANCE, 
> +                                      stringarraypropname,
> +                                      &arr,
> +                                      CMPI_stringA);
> +                        
> +                        free(stringarraypropname);
> +                        }
> +        ;
> +
>  /* END OF RULES SECTION */
>  %%
> 
>  /* USER SUBROUTINE SECTION */
> 
> +inline void ins_chars_into_cmstr_arr(const CMPIBroker *broker,
> +                                     CMPIArray *arr,
> +                                     CMPICount index,
> +                                     char *str)

Instead of including this function in eo_util_parser.y, I would put it 
in eo_parser.c.  This is similar to set_int_prop() - it's called in 
set_int_prop, but defined in eo_parser.c.


> +{
> +        CMPIString *cm_str;
> +        CMPIStatus s;
> +        
> +        cm_str = CMNewString(_BROKER, str, &s);
> +        if (s.rc != CMPI_RC_OK || CMIsNullObject(cm_str))
> +                EOTRACE("Error creating CMPIString");

I would return an error here.  Instead of EOTRACE, CU_DEBUG().

> +
> +        s = CMSetArrayElementAt(arr, index, &cm_str, CMPI_string);
> +        if (s.rc != CMPI_RC_OK)
> +                EOTRACE("Error setting array element %u\n"
> +                        "Error code: %d\n", index, s.rc);

Same here.

-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com