Re: [PATCH 1/7] ndctl: cxl: add helper function to parse trace event to json object

[Dave Jiang <dave.jiang@intel.com>]

On 10/31/2022 1:01 PM, Alison Schofield wrote:
> On Wed, Sep 14, 2022 at 01:47:55PM -0700, Dave Jiang wrote:
>> Add the helper function that parses a trace event captured by
>> libtraceevent in a tep handle. All the parsed fields are added to a json
>> object. The json object is added to the provided list in the input parameter.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> Hi Dave,
> 
> I've been using these parsing patches, and it all works for the purpose
> of parsing the cxl device poison list.
> 
> Tested-by: Alison Schofield <alison.schofield@intel.com>

Thanks!

> 
> One comment below...
> 
> -snip-
> 
>> ---
>> +
>> +	for (i = 0; fields[i]; i++) {
>> +		struct tep_format_field *f = fields[i];
>> +		int len;
>> +		char *tmp;
>> +
>> +		tmp = strcasestr(f->type, "char[]");
>> +		if (tmp) { /* event field is a string */
>> +			char *str;
>> +
>> +			str = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
>> +			if (!str)
>> +				continue;
>> +
>> +			jobj = json_object_new_string(str);
>> +			if (!jobj) {
>> +				rc = -ENOMEM;
>> +				goto err;
>> +			}
>> +
>> +			json_object_object_add(jevent, f->name, jobj);
>> +		} else if (f->arraylen) { /* data array */
>> +			unsigned char *data;
>> +			int chunks;
>> +
>> +			data = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
>> +			if (!data)
>> +				continue;
>> +
>> +			/* check to see if we have a UUID */
>> +			tmp = strcasestr(f->name, "uuid");
>> +			if (tmp && f->arraylen == 16) {
>> +				char uuid[SYSFS_ATTR_SIZE];
>> +
>> +				uuid_unparse(data, uuid);
>> +				jobj = json_object_new_string(uuid);
>> +				if (!jobj) {
>> +					rc = -ENOMEM;
>> +					goto err;
>> +				}
>> +
>> +				json_object_object_add(jevent, f->name, jobj);
>> +				continue;
>> +			}
> 
> Insted of comparing the field name, is it possible to check for
> f->type of 'uuid_t'?
> 

Yes. I talked to Ira and he'll change his uuid to uuid_t from u8[]. That 
should make it better. Thanks for pointing that out.

> 
>> +
>> +			jarray = json_object_new_array();
>> +			if (!jarray) {
>> +				rc = -ENOMEM;
>> +				goto err;
>> +			}
>> +
>> +			chunks = f->size / f->elementsize;
>> +			for (j = 0; j < chunks; j++) {
>> +				jobj = num_to_json(data, f->elementsize);
>> +				if (!jobj) {
>> +					json_object_put(jarray);
>> +					return -ENOMEM;
>> +				}
>> +				json_object_array_add(jarray, jobj);
>> +				data += f->elementsize;
>> +			}
>> +
>> +			json_object_object_add(jevent, f->name, jarray);
>> +		} else { /* single number */
>> +			char *data;
>> +
>> +			data = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
>> +			if (!data)
>> +				continue;
>> +
>> +			jobj = num_to_json(data, f->elementsize);
>> +			if (!jobj) {
>> +				rc = -ENOMEM;
>> +				goto err;
>> +			}
>> +
>> +			json_object_object_add(jevent, f->name, jobj);
>> +		}
>> +	}
>> +
>> +	list_add_tail(jlist_head, &jnode->list);
>> +	return 0;
>> +
>> +err:
>> +	json_object_put(jevent);
>> +err_jevent:
>> +	free(jnode);
>> +	return rc;
>> +}
> 
> snip
>