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 *);
See attachment.
Michal
If you really dislike the macro and want to put in direct code at least
do this:
/* align to char * pointers sizes */
pad = (sizeof(char *) - (i % sizeof(char *)
pad %= sizeof(char *)
HTH,
Simo.
>From bda31af705b599bce839ed762cf3eed7598a3026 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..d609c4a 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 = CALCULATE_PAD(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..d146c69 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 = CALCULATE_PAD(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..2dd7f24 100644
--- a/src/util/util_safealign.h
+++ b/src/util/util_safealign.h
@@ -32,6 +32,9 @@
#include <string.h>
#include <stdint.h>
+#define CALCULATE_PAD(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
