[SSSD] [PATCH] SDAP: rem warning - sizelimit exceeded in POSIX check
Jakub Hrozek
jhrozek at redhat.com
Sun Oct 11 09:06:47 UTC 2015
On Fri, Oct 09, 2015 at 08:02:08PM +0200, Michal Židek wrote:
> On 10/09/2015 02:05 PM, Lukas Slebodnik wrote:
> >On (09/10/15 13:56), Jakub Hrozek wrote:
> >>On Tue, Oct 06, 2015 at 01:25:33PM +0200, Pavel Reichl wrote:
> >>>
> >>>
> >>>On 10/06/2015 11:21 AM, Jakub Hrozek wrote:
> >>>>
> >>>>I personally don't care about unsigned vs unsigned int, but see my
> >>>>comment about the request inline..
> >>>>
> >>>>> From 2281410185205ab3fc483f4c45b1b1378b62c331 Mon Sep 17 00:00:00 2001
> >>>>>From: Pavel Reichl <preichl at redhat.com>
> >>>>>Date: Fri, 25 Sep 2015 07:05:30 -0400
> >>>>>Subject: [PATCH] SDAP: rem warning - sizelimit exceeded in POSIX check
> >>>>>
> >>>>>Add new parameter 'flags' to sdap_get_generic_ext_send_ext() which can
> >>>>>be set to suppress warning about 'sizelimit exceeded'.
> >>>>>
> >>>>>Resolves:
> >>>>>https://fedorahosted.org/sssd/ticket/2804
> >>>>>---
> >>>>> src/providers/ldap/sdap_async.c | 78 +++++++++++++++++++++++++++++------------
> >>>>> 1 file changed, 56 insertions(+), 22 deletions(-)
> >>>>>
> >>>>>diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
> >>>>>index 97c9ea5df61a6516ca74bb73edc9a116b1266c71..0044ea9c319af08a87fa59e89559851ad6e8abc5 100644
> >>>>>--- a/src/providers/ldap/sdap_async.c
> >>>>>+++ b/src/providers/ldap/sdap_async.c
> >>>>>@@ -1164,6 +1164,8 @@ struct sdap_get_generic_ext_state {
> >>>>> void *cb_data;
> >>>>>
> >>>>> bool allow_paging;
> >>>>>+
> >>>>>+ unsigned int flags;
> >>>>> };
> >>>>>
> >>>>> static errno_t sdap_get_generic_ext_step(struct tevent_req *req);
> >>>>>@@ -1172,23 +1174,26 @@ static void sdap_get_generic_op_finished(struct sdap_op *op,
> >>>>> struct sdap_msg *reply,
> >>>>> int error, void *pvt);
> >>>>>
> >>>>>+#define WARN_SIZELIMIT 1
> >>>>>+
> >>>>> static struct tevent_req *
> >>>>>-sdap_get_generic_ext_send(TALLOC_CTX *memctx,
> >>>>>- struct tevent_context *ev,
> >>>>>- struct sdap_options *opts,
> >>>>>- struct sdap_handle *sh,
> >>>>>- const char *search_base,
> >>>>>- int scope,
> >>>>>- const char *filter,
> >>>>>- const char **attrs,
> >>>>>- int attrsonly,
> >>>>>- LDAPControl **serverctrls,
> >>>>>- LDAPControl **clientctrls,
> >>>>>- int sizelimit,
> >>>>>- int timeout,
> >>>>>- bool allow_paging,
> >>>>>- sdap_parse_cb parse_cb,
> >>>>>- void *cb_data)
> >>>>>+sdap_get_generic_ext_send_ext(TALLOC_CTX *memctx,
> >>>>>+ struct tevent_context *ev,
> >>>>>+ struct sdap_options *opts,
> >>>>>+ struct sdap_handle *sh,
> >>>>>+ const char *search_base,
> >>>>>+ int scope,
> >>>>>+ const char *filter,
> >>>>>+ const char **attrs,
> >>>>>+ int attrsonly,
> >>>>>+ LDAPControl **serverctrls,
> >>>>>+ LDAPControl **clientctrls,
> >>>>>+ int sizelimit,
> >>>>>+ int timeout,
> >>>>>+ bool allow_paging,
> >>>>>+ sdap_parse_cb parse_cb,
> >>>>>+ void *cb_data,
> >>>>>+ unsigned int flags)
> >>>>
> >>>>I don't like us adding another request below sdap_get_generic_ext_send.
> >>>
> >>>OK, I understand that.
> >>>
> >>>>IIRC the idea was that sdap_get_generic_ext_send is really raw and
> >>>>unless you need the raw access you should use sdap_get_generic_send
> >>>>(without _ext).
> >>>>
> >>>>So here I would prefer to return ERR_SIZELIMIT_EXCEEDED from the _ext request
> >>>>and let callers deal with the error. The posix check caller would ignore
> >>>>it, the others might display an error and carry on.
> >>>This sound to me like code duplication. I would have to add to several calls same check and print same message.
> >>
> >>This is totally untested version of what I meant:
> >> https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/commit/?h=misc
> >>
> >It looks more elegant then adding flags to function.
> >So +1
> >
> >But there is still a change that it does not work because patches are totally
> >untested :-)
> >
> >LS
>
> I agree with Pavel that ignoring the debug message is non standard
> thing that we want to do only during the posix check and it does not
> add much readbility/ease of use when the caller needs to stalk for
> the special error code (only in one case it will be ignored).
> I also agree with Jakub that we do not want to expand already big
> number of arguments.
>
> Please see the attached patch. It is less invasive and only changes
> parameters for the posix check (for now the one and only special
> case when we want to ignore the debug message).
>
> Michal
>
> PS: The patch is also totally untested and serves for illustration,
> but it is simple enough to accidentally work :)
LGTM (not tested).
More information about the sssd-devel
mailing list