On 8/17/26 2:34 PM, Ilya Maximets wrote:
> On 8/17/26 1:59 PM, Dumitru Ceara wrote:
>>
>> The leak happened when resetting index fields. This really makes me
>> wonder if we shouldn't harden the IDL index implementation to allow
>> users to actually reset fields instead of having to use a completely new
>> filter.
>>
>> It's not really obvious from the API I guess. CC-ing Ilya for his
>> thoughts too on this matter.
>
> Feels like there is a bug in this code:
>
Yep.
> void
> ovsdb_idl_index_write(struct ovsdb_idl_row *const_row,
> const struct ovsdb_idl_column *column,
> struct ovsdb_datum *datum,
> const struct ovsdb_idl_table_class *class)
> {
> struct ovsdb_idl_row *row = CONST_CAST(struct ovsdb_idl_row *, const_row);
> size_t column_idx = column - class->columns;
>
> if (bitmap_is_set(row->written, column_idx)) {
> free(row->new_datum[column_idx].values);
> free(row->new_datum[column_idx].keys);
AFAICT these two lines above should be replaced by:
ovsdb_datum_destroy(&row->new_datum[column_idx], &column->type);
I'll see if I can post a patch soon.
> } else {
> bitmap_set1(row->written, column_idx);
> }
> row->new_datum[column_idx] = *datum;
> (column->unparse)(row);
> (column->parse)(row, &row->new_datum[column_idx]);
> }
>
> The bitmap_set1() should probably be unconditional. But I didn't look
> in too deep.
Uhm, I don't think that's needed, if it's 1 there's no reason to set it
to 1 again. But it was the missing proper destroy call that's causing
this as mentioned above.
>
> Best regards, Ilya Maximets.
>
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
