On 12/03/2013 05:04 PM, Simo Sorce wrote:
On Tue, 2013-12-03 at 16:03 +0100, Michal Židek wrote:
On 12/03/2013 02:34 PM, Simo Sorce wrote:
On Tue, 2013-12-03 at 13:28 +0100, Michal Židek wrote:
On 12/03/2013 10:57 AM, Lukas Slebodnik wrote:
I thik we can change while loop with som expresion.
pad = i % sizeof(char *)
pad = (pad != 0) ? sizeof(char *) - pad : 0;
Ok, I added this change to the patch. I did not want to change the code
much with these patches, the purpose was to just fix the alignment
issues. But this is very small change and additional patch would just
change the same lines. So I think it is ok to include.
or something nicer :-) (maybe macro?)
I do not think that obfuscation with a macro is appropriate here.
Actually the code is pretty hard to read and a macro would avoid
duplication and make it clear what the purpose is:
Something like:
#define PTR_SIZE sizeof(char *)
#define ALIGN_PTR_PAD(base, padding) \
padding = (PTR_SIZE - (base % PTR_SIZE)) % PTR_SIZE)
Then in the code just call:
ALIGN_PTR_PAD(i, pad)
I tend to dislike macros, but maybe we will need similar pattern on
other places in the future as well, so putting this into macro may not
be as bad idea as I originally though.
However I would propose a different macro, that would be used like this:
pad = CALCULATE_PAD(i, char *);
Maybe PADDING_SIZE() then.
Simo.
Patch with changed macro name attached.
Michal
>From 37f1f8f067a33321af420924f15031cd5dd7733a Mon Sep 17 00:00:00 2001
From: Michal Zidek <[email protected]>
Date: Tue, 10 Sep 2013 23:09:04 +0200
Subject: [PATCH] Properly align buffer when storing pointers.
Properly align buffer address to sizeof(char *) when storing
pointers to strings.
resolves:
https://fedorahosted.org/sssd/ticket/1359
---
src/sss_client/nss_group.c | 8 +++-----
src/sss_client/nss_mc_group.c | 3 +++
src/sss_client/nss_services.c | 7 ++-----
src/util/util_safealign.h | 3 +++
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/sss_client/nss_group.c b/src/sss_client/nss_group.c
index a7fb093..9e25931 100644
--- a/src/sss_client/nss_group.c
+++ b/src/sss_client/nss_group.c
@@ -233,14 +233,12 @@ static int sss_nss_getgr_readrep(struct sss_nss_gr_rep *pr,
NULL);
if (ret != EOK) return ret;
- /* Make sure pr->buffer[i+pad] is 32 bit aligned */
- pad = 0;
- while((i + pad) % 4) {
- pad++;
- }
+ /* Make sure pr->buffer[i+pad] is aligned to sizeof(char *) */
+ pad = PADDING_SIZE(i, char *);
/* now members */
pr->result->gr_mem = (char **)&(pr->buffer[i+pad]);
+
ptmem = (sizeof(char *) * (mem_num + 1)) + pad;
if (ptmem > dlen) {
return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index 5610233..c9870ab 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -64,7 +64,10 @@ static errno_t sss_nss_mc_parse_result(struct sss_mc_rec *rec,
/* fill in group */
result->gr_gid = data->gid;
+
+ /* The address &buffer[0] must be aligned to sizeof(char *) */
result->gr_mem = (char **)buffer;
+
result->gr_mem[data->members] = NULL;
cookie = NULL;
diff --git a/src/sss_client/nss_services.c b/src/sss_client/nss_services.c
index b40e1fa..e89e0d2 100644
--- a/src/sss_client/nss_services.c
+++ b/src/sss_client/nss_services.c
@@ -127,11 +127,8 @@ sss_nss_getsvc_readrep(struct sss_nss_svc_rep *sr,
NULL);
if (ret != EOK) return ret;
- /* Make sure sr->buffer[i+pad] is 32-bit aligned */
- pad = 0;
- while((i + pad) % 4) {
- pad++;
- }
+ /* Make sure sr->buffer[i+pad] is aligned to sizeof(char *) */
+ pad = PADDING_SIZE(i, char *);
/* Copy in the aliases */
sr->result->s_aliases = (char **) &(sr->buffer[i+pad]);
diff --git a/src/util/util_safealign.h b/src/util/util_safealign.h
index c39ba15..d8780f7 100644
--- a/src/util/util_safealign.h
+++ b/src/util/util_safealign.h
@@ -32,6 +32,9 @@
#include <string.h>
#include <stdint.h>
+#define PADDING_SIZE(base, type) \
+ ((sizeof(type) - ((base) % sizeof(type))) % sizeof(type))
+
#define SIZE_T_OVERFLOW(current, add) \
(((size_t)(add)) > (SIZE_MAX - ((size_t)(current))))
--
1.7.11.2
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
