Dne 29.11.2011 14:40, Jan Zelený napsal(a):
patch #0001: Nack, first of all, I still don't like setting *_filter to NULL on the beginning of the helper function. In case the function fails, the content of this variable should not be tampered with.
In your comment in the helper function, you assume that comma can be in the attribute name. I'm don't think this is allowed by RFC. Is there any other scenario this "if" is valid for? If not I suggest removing it entirely. If there is, please modify it to something like this:
if (len_diff>= 2&& dn[len_diff-1] == ','&& dn[len_diff-2] == '\')
patch #0002: Nack, regarding our previous conversation in the original thread, I'm pretty sure, returning ret is better than returning EOK every time. Also I'm pretty sure that if you return other return code than EAGAIN, the tevent_req_error() will be called later.
patch #0003: Ack, for any other who would like to review this patch, please note that the change only limits number of queries going to LDAP server. It will not eliminate all queries due to the filtering being done by an (|....) filter.
Thanks Jan
New patches attached.
Ack to all three. I did only differential review between this and previous set of patches but I tested those patches before.
Jan