[pacman-dev] [PATCH] Consolidate and improve table implementations

[Simon Gomizelj]

>>  
>>  int trans_init(alpm_transflag_t flags, int check_valid)
>> @@ -424,6 +432,60 @@ static size_t string_length(const char *s)
>>  	return len;
>>  }
>>  
>> +static void add_table_cell(alpm_list_t **row, char *label, int mode)
>> +{
>> +	struct table_cell_t *cell = malloc(sizeof(struct table_cell_t));
>> +
>> +	cell->label = label;
>> +	cell->mode = mode;
>> +	cell->len = string_length(label);
>> +
>> +	*row = alpm_list_add(*row, cell);
>> +}
>> +
>> +static void table_free_cell(void *ptr)
>> +{
>> +	struct table_cell_t *cell = ptr;
>> +
>> +	if(cell) {
>> +		if((cell->mode & CELL_FREE) && cell->label) {
>> +			free(cell->label);
> 
> This isn't an official pacman style guideline and we do it in other
> places, but I personally view NULL checks before calling free functions
> as useless noise.  *free() functions should generally be NULL-safe, so
> manually checking adds unnecessary clutter and can be confusing when
> done inconsistently.
> 

Its not NULL safe though. If cell is NULL, then cell->mode / cell->label
is a null dereference error.

>> +		}
>> +		free(cell);
>> +	}
>> +}
>> +
>> +static void table_free(alpm_list_t *headers, alpm_list_t *rows)
>> +{
>> +	alpm_list_t *i;
>> +
>> +	alpm_list_free_inner(headers, table_free_cell);
>> +
>> +	for(i = rows; i; i = alpm_list_next(i)) {
>> +		if(i->data) {
>> +			alpm_list_free_inner(i->data, table_free_cell);
>> +			alpm_list_free(i->data);
> 
> NULL check.
> 

I'm assuming you don't want the check then?