On 06/30/13 at 10:53pm, Simon Gomizelj wrote:
> >>
> >> 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.
>
I was referring to the cell->label check, not the if(cell)...
> >> + }
> >> + 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?
>
