#47506 slapi_valueset_add_value_ext must return int
Closed: wontfix 3 years ago by spichugi. Opened 10 years ago by rmeggins.

If slapi_valueset_add_value_ext is used with the SLAPI_VALUE_FLAG_DUPCHECK flag, the caller is responsible for knowing how to cleanup the given value if there is an error. For example, if the flags are SLAPI_VALUE_FLAG_DUPCHECK|SLAPI_VALUE_FLAG_PASSIN, slapi_valueset_add_value_ext does not know if it needs to free the given value or not. The caller should be responsible.
Right now, no code in 389 uses SLAPI_VALUE_FLAG_DUPCHECK|SLAPI_VALUE_FLAG_PASSIN, but since slapi_valueset_add_value_ext is part of the public SLAPI, someone could use it and cause a memory leak or other problem.


this ticket has unfortunately been postponed again and again.

the request is valid, but what are the tasks ?

  • fix the code

  • we need to update the plugin guide documenting the return code and the new flag for DUPCHECK

  • we need to change the public api, what is the effect on existing plugins ?

Replying to [comment:9 lkrispen]:

this ticket has unfortunately been postponed again and again.

the request is valid, but what are the tasks ?

  • fix the code

Yes.

  • we need to update the plugin guide documenting the return code and the new flag for DUPCHECK

Yes.

  • we need to change the public api, what is the effect on existing plugins ?

Need to check IPA plugins, but I doubt this is being used at all, so probably no effect.

I searched the 389-ds-base source code and found slapi_valueset_add_value_ext is not called with SLAPI_VALUE_FLAG_DUPCHECK, but slapi_valueset_add_attr_value_ext and slapi_valueset_add_attr_valuearray_ext are.
{{{
slapi-plugin.h:void slapi_valueset_add_value_ext(Slapi_ValueSet vs, const Slapi_Value addval, unsigned long flags);
slapi-plugin.h:int slapi_valueset_add_attr_value_ext(const Slapi_Attr a, Slapi_ValueSet vs, Slapi_Value addval, unsigned long flags);
slapi-private.h:int slapi_valueset_add_attr_valuearray_ext(const Slapi_Attr
a, Slapi_ValueSet vs, Slapi_Value addval, int nvals, unsigned long flags, int dup_index);
}}}

slapi_valueset_add_value_ext internally calls slapi_valueset_add_attr_valuearray_ext, in which error could occur only if SLAPI_VALUE_FLAG_DUPCHECK is passed. That said, slapi_valueset_add_value_ext never fails in the current code.

Theoretically, SLAPI_VALUE_FLAG_DUPCHECK can be passed to slapi_valueset_add_value_ext and the return code is supposed to handled correctly (the preliminary patch is attached). But it may not be worth changing in this version since there is no positive gain...

Pushing it to 1.4.

Metadata Update from @rmeggins:
- Issue assigned to lkrispen
- Issue set to the milestone: 1.4 backlog

7 years ago

Metadata Update from @mreynolds:
- Custom field reviewstatus adjusted to None
- Issue close_status updated to: None
- Issue set to the milestone: 1.4.4 (was: 1.4 backlog)

3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/843

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix
- Issue status updated to: Closed (was: Open)

3 years ago

Login to comment on this ticket.

Metadata