[Freeipa-devel] [PATCHES 134-136] extdom: handle ERANGE return code for getXXYYY_r()

[Alexander Bokovoy]

On Mon, 02 Mar 2015, Sumit Bose wrote:
>diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
>index 20fdd62b20f28f5384cf83b8be5819f721c6c3db..84aeb28066f25f05a89d0c2d42e8b060e2399501 100644
>--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
>+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
>@@ -49,6 +49,220 @@
>
> #define MAX(a,b) (((a)>(b))?(a):(b))
> #define SSSD_DOMAIN_SEPARATOR '@'
>+#define MAX_BUF (1024*1024*1024)
>+
>+
>+
>+static int get_buffer(size_t *_buf_len, char **_buf)
>+{
>+    long pw_max;
>+    long gr_max;
>+    size_t buf_len;
>+    char *buf;
>+
>+    pw_max = sysconf(_SC_GETPW_R_SIZE_MAX);
>+    gr_max = sysconf(_SC_GETGR_R_SIZE_MAX);
>+
>+    if (pw_max == -1 && gr_max == -1) {
>+        buf_len = 16384;
>+    } else {
>+        buf_len = MAX(pw_max, gr_max);
>+    }
Here you'd get buf_len equal to 1024 by default on Linux which is too
low for our use case. I think it would be beneficial to add one more
MAX(buf_len, 16384):
-    if (pw_max == -1 && gr_max == -1) {
-        buf_len = 16384;
-    } else {
-        buf_len = MAX(pw_max, gr_max);
-    }
+    buf_len = MAX(16384, MAX(pw_max, gr_max));

with MAX(MAX(),..) you also get rid of if() statement as resulting
rvalue would be guaranteed to be positive.

The rest is going along the common lines but would it be better to
allocate memory once per LDAP client request rather than always ask for
it per each NSS call? You can guarantee a sequential use of the buffer
within the LDAP client request processing so there is no problem with
locks but having this memory re-allocated on subsequent
getpwnam()/getpwuid()/... calls within the same request processing seems
suboptimal to me.

-- 
/ Alexander Bokovoy