Alignment issue of iobuf functions
NIIBE Yutaka
gniibe at fsij.org
Fri Jan 8 03:16:10 CET 2016
Hello, again,
On 01/06/2016 07:36 PM, Neal H. Walfield wrote:
> Unfortunately, we are overloading the buf parameter too much.
Yesterday's version which changes byte * to void * was lost in the SMTP
communication. I make another version, which avoids overloading.
Instead of returning the pointer to string, it fills the description
to the buffer. Since IOBUFCTRL_DESC is only used for debugging
purpose, this small change of behavior is OK, I suppose.
The API of the function iobuf_desc is somewhat ugly. And I don't like
strncpy in general, though. Since it is an internal function, the
impact of iobuf_desc ugliness is limited to common/iobuf.c. I think
that we can pay something like that for the justice of compiler type
checking and its benefits.
Since this change is needed to GnuPG 1.4 and 2.0 as well, I'd like
to confirm if this patch is OK to commit.
For me, I prefer this one to the change (byte *) -> (void *), even
with small change of runtime behavior.
diff --git a/common/iobuf.c b/common/iobuf.c
index d149e2e..d12252c 100644
--- a/common/iobuf.c
+++ b/common/iobuf.c
@@ -578,7 +578,7 @@ file_filter (void *opaque, int control, iobuf_t
chain, byte * buf,
}
else if (control == IOBUFCTRL_DESC)
{
- *(char **) buf = "file_filter(fd)";
+ strncpy (buf, "file_filter(fd)", *ret_len);
}
else if (control == IOBUFCTRL_FREE)
{
@@ -667,7 +667,7 @@ file_es_filter (void *opaque, int control, iobuf_t
chain, byte * buf,
}
else if (control == IOBUFCTRL_DESC)
{
- *(char **) buf = "estream_filter";
+ strncpy (buf, "estream_filter", *ret_len);
}
else if (control == IOBUFCTRL_FREE)
{
@@ -765,7 +765,7 @@ sock_filter (void *opaque, int control, iobuf_t
chain, byte * buf,
}
else if (control == IOBUFCTRL_DESC)
{
- *(char **) buf = "sock_filter";
+ strncpy (buf, "sock_filter", *ret_len);
}
else if (control == IOBUFCTRL_FREE)
{
@@ -993,7 +993,7 @@ block_filter (void *opaque, int control, iobuf_t
chain, byte * buffer,
}
else if (control == IOBUFCTRL_DESC)
{
- *(char **) buf = "block_filter";
+ strncpy (buf, "block_filter", *ret_len);
}
else if (control == IOBUFCTRL_FREE)
{
@@ -1057,19 +1057,25 @@ block_filter (void *opaque, int control, iobuf_t
chain, byte * buffer,
return rc;
}
+#define MAX_IOBUF_DESC 32
+/*
+ * Fill the buffer by the description of iobuf A.
+ * The buffer size should be MAX_IOBUF_DESC (or larger).
+ * Returns BUF as (const char *).
+ */
static const char *
-iobuf_desc (iobuf_t a)
+iobuf_desc (iobuf_t a, byte *buf)
{
- size_t dummy_len = 0;
- const char *desc = "?";
+ size_t len = MAX_IOBUF_DESC - 1;
- if (! a || ! a->filter)
- return desc;
+ buf[MAX_IOBUF_DESC - 1] = '\0';
- a->filter (a->filter_ov, IOBUFCTRL_DESC, NULL,
- (byte *) & desc, &dummy_len);
+ if (! a || ! a->filter)
+ memcpy (buf, "?", 2);
+ else
+ a->filter (a->filter_ov, IOBUFCTRL_DESC, NULL, buf, &len);
- return desc;
+ return buf;
}
static void
@@ -1079,9 +1085,10 @@ print_chain (iobuf_t a)
return;
for (; a; a = a->chain)
{
+ byte desc[MAX_IOBUF_DESC];
log_debug ("iobuf chain: %d.%d '%s' filter_eof=%d start=%d len=%d\n",
- a->no, a->subno, iobuf_desc (a), a->filter_eof,
+ a->no, a->subno, iobuf_desc (a, desc), a->filter_eof,
(int) a->d.start, (int) a->d.len);
}
}
@@ -1126,6 +1133,7 @@ iobuf_close (iobuf_t a)
for (; a; a = a_chain)
{
+ byte desc[MAX_IOBUF_DESC];
int rc2 = 0;
a_chain = a->chain;
@@ -1135,7 +1143,7 @@ iobuf_close (iobuf_t a)
if (DBG_IOBUF)
log_debug ("iobuf-%d.%d: close '%s'\n",
- a->no, a->subno, iobuf_desc (a));
+ a->no, a->subno, iobuf_desc (a, desc));
if (a->filter && (rc2 = a->filter (a->filter_ov, IOBUFCTRL_FREE,
a->chain, NULL, &dummy_len)))
@@ -1275,6 +1283,7 @@ do_open (const char *fname, int special_filenames,
size_t len = 0;
int print_only = 0;
int fd;
+ byte desc[MAX_IOBUF_DESC];
assert (use == IOBUF_INPUT || use == IOBUF_OUTPUT);
@@ -1321,7 +1330,7 @@ do_open (const char *fname, int special_filenames,
file_filter (fcx, IOBUFCTRL_INIT, NULL, NULL, &len);
if (DBG_IOBUF)
log_debug ("iobuf-%d.%d: open '%s' desc=%s fd=%d\n",
- a->no, a->subno, fname, iobuf_desc (a), FD2INT (fcx->fp));
+ a->no, a->subno, fname, iobuf_desc (a, desc), FD2INT (fcx->fp));
return a;
}
@@ -1439,6 +1448,8 @@ iobuf_sockopen (int fd, const char *mode)
int
iobuf_ioctl (iobuf_t a, iobuf_ioctl_t cmd, int intval, void *ptrval)
{
+ byte desc[MAX_IOBUF_DESC];
+
if (cmd == IOBUF_IOCTL_KEEP_OPEN)
{
/* Keep system filepointer/descriptor open. This was used in
@@ -1446,7 +1457,7 @@ iobuf_ioctl (iobuf_t a, iobuf_ioctl_t cmd, int
intval, void *ptrval)
anymore. */
if (DBG_IOBUF)
log_debug ("iobuf-%d.%d: ioctl '%s' keep_open=%d\n",
- a ? a->no : -1, a ? a->subno : -1, iobuf_desc (a),
+ a ? a->no : -1, a ? a->subno : -1, iobuf_desc (a, desc),
intval);
for (; a; a = a->chain)
if (!a->chain && a->filter == file_filter)
@@ -1480,7 +1491,7 @@ iobuf_ioctl (iobuf_t a, iobuf_ioctl_t cmd, int
intval, void *ptrval)
{
if (DBG_IOBUF)
log_debug ("iobuf-%d.%d: ioctl '%s' no_cache=%d\n",
- a ? a->no : -1, a ? a->subno : -1, iobuf_desc (a),
+ a ? a->no : -1, a ? a->subno : -1, iobuf_desc (a, desc),
intval);
for (; a; a = a->chain)
if (!a->chain && a->filter == file_filter)
@@ -1658,8 +1669,9 @@ iobuf_push_filter2 (iobuf_t a,
if (DBG_IOBUF)
{
+ byte desc[MAX_IOBUF_DESC];
log_debug ("iobuf-%d.%d: push '%s'\n",
- a->no, a->subno, iobuf_desc (a));
+ a->no, a->subno, iobuf_desc (a, desc));
print_chain (a);
}
@@ -1681,10 +1693,11 @@ pop_filter (iobuf_t a, int (*f) (void *opaque,
int control,
iobuf_t b;
size_t dummy_len = 0;
int rc = 0;
+ byte desc[MAX_IOBUF_DESC];
if (DBG_IOBUF)
log_debug ("iobuf-%d.%d: pop '%s'\n",
- a->no, a->subno, iobuf_desc (a));
+ a->no, a->subno, iobuf_desc (a, desc));
if (a->use == IOBUF_INPUT_TEMP || a->use == IOBUF_OUTPUT_TEMP)
{
/* This should be the last filter in the pipeline. */
@@ -2188,6 +2201,7 @@ iobuf_write_temp (iobuf_t dest, iobuf_t source)
size_t
iobuf_temp_to_buffer (iobuf_t a, byte * buffer, size_t buflen)
{
+ byte desc[MAX_IOBUF_DESC];
size_t n;
while (1)
@@ -2195,7 +2209,7 @@ iobuf_temp_to_buffer (iobuf_t a, byte * buffer,
size_t buflen)
int rc = filter_flush (a);
if (rc)
log_bug ("Flushing iobuf %d.%d (%s) from iobuf_temp_to_buffer failed.
Ignoring.\n",
- a->no, a->subno, iobuf_desc (a));
+ a->no, a->subno, iobuf_desc (a, desc));
if (! a->chain)
break;
a = a->chain;
diff --git a/common/iobuf.h b/common/iobuf.h
index cb79105..69764d6 100644
--- a/common/iobuf.h
+++ b/common/iobuf.h
@@ -404,10 +404,10 @@ int iobuf_cancel (iobuf_t iobuf);
called on the pipeline.
IOBUFCTRL_DESC: Called with this value to get a human-readable
- description of the filter. * (char **) BUF should set to the
- NUL-terminated string. Note: you need to keep track of this
- value and, if necessary, free it when the filter function is
- called with control set to IOBUFCTRL_FREE.
+ description of the filter. *LEN is the size of the buffer.
+ The description is filled into BUF, NUL-terminated. Always
+ returns 0. When the size of the buffer is shorter than the
+ description, it is truncated and not NUL-terminated.
*/
int iobuf_push_filter (iobuf_t a, int (*f) (void *opaque, int control,
iobuf_t chain, byte * buf,
diff --git a/common/t-iobuf.c b/common/t-iobuf.c
index 99581b9..4b5c935 100644
--- a/common/t-iobuf.c
+++ b/common/t-iobuf.c
@@ -16,7 +16,7 @@ every_other_filter (void *opaque, int control,
if (control == IOBUFCTRL_DESC)
{
- *(char **) buf = "every_other_filter";
+ strncpy (buf, "every_other_filter", *len);
}
if (control == IOBUFCTRL_UNDERFLOW)
{
@@ -52,7 +52,7 @@ double_filter (void *opaque, int control,
if (control == IOBUFCTRL_DESC)
{
- * (char **) buf = "double_filter";
+ strncpy (buf, "double_filter", *len);
}
if (control == IOBUFCTRL_FLUSH)
{
diff --git a/g10/armor.c b/g10/armor.c
index 6c133a2..e468e7e 100644
--- a/g10/armor.c
+++ b/g10/armor.c
@@ -1251,7 +1251,7 @@ armor_filter( void *opaque, int control,
release_armor_context (afx);
}
else if( control == IOBUFCTRL_DESC )
- *(char**)buf = "armor_filter";
+ strncpy (buf, "armor_filter", *ret_len);
return rc;
}
diff --git a/g10/cipher.c b/g10/cipher.c
index b72b144..d47d52b 100644
--- a/g10/cipher.c
+++ b/g10/cipher.c
@@ -157,7 +157,7 @@ cipher_filter( void *opaque, int control,
gcry_cipher_close (cfx->cipher_hd);
}
else if( control == IOBUFCTRL_DESC ) {
- *(char**)buf = "cipher_filter";
+ strncpy (buf, "cipher_filter", *ret_len);
}
return rc;
}
diff --git a/g10/compress-bz2.c b/g10/compress-bz2.c
index ea80956..d684c51 100644
--- a/g10/compress-bz2.c
+++ b/g10/compress-bz2.c
@@ -248,6 +248,6 @@ compress_filter_bz2( void *opaque, int control,
zfx->release (zfx);
}
else if( control == IOBUFCTRL_DESC )
- *(char**)buf = "compress_filter";
+ strncpy (buf, "compress_filter", *ret_len);
return rc;
}
diff --git a/g10/compress.c b/g10/compress.c
index 8047dbb..e5819db 100644
--- a/g10/compress.c
+++ b/g10/compress.c
@@ -288,7 +288,7 @@ compress_filter( void *opaque, int control,
zfx->release (zfx);
}
else if( control == IOBUFCTRL_DESC )
- *(char**)buf = "compress_filter";
+ strncpy (buf, "compress_filter", *ret_len);
return rc;
}
#endif /*HAVE_ZIP*/
diff --git a/g10/decrypt-data.c b/g10/decrypt-data.c
index 2d9f54f..a46f366 100644
--- a/g10/decrypt-data.c
+++ b/g10/decrypt-data.c
@@ -425,7 +425,7 @@ mdc_decode_filter (void *opaque, int control, IOBUF a,
}
else if ( control == IOBUFCTRL_DESC )
{
- *(char**)buf = "mdc_decode_filter";
+ strncpy (buf, "mdc_decode_filter", *ret_len);
}
return rc;
}
@@ -496,7 +496,7 @@ decode_filter( void *opaque, int control, IOBUF a,
byte *buf, size_t *ret_len)
}
else if ( control == IOBUFCTRL_DESC )
{
- *(char**)buf = "decode_filter";
+ strncpy (buf, "decode_filter", *ret_len);
}
return rc;
}
diff --git a/g10/encrypt.c b/g10/encrypt.c
index eca1c27..afeaf28 100644
--- a/g10/encrypt.c
+++ b/g10/encrypt.c
@@ -848,7 +848,7 @@ encrypt_filter (void *opaque, int control,
}
else if ( control == IOBUFCTRL_DESC )
{
- *(char**)buf = "encrypt_filter";
+ strncpy (buf, "encrypt_filter", *ret_len);
}
return rc;
}
diff --git a/g10/mdfilter.c b/g10/mdfilter.c
index 708bdcd..2c3b2ec 100644
--- a/g10/mdfilter.c
+++ b/g10/mdfilter.c
@@ -58,7 +58,7 @@ md_filter( void *opaque, int control,
*ret_len = i;
}
else if( control == IOBUFCTRL_DESC )
- *(char**)buf = "md_filter";
+ strncpy (buf, "md_filter", *ret_len);
return rc;
}
diff --git a/g10/progress.c b/g10/progress.c
index ca20223..e8159e9 100644
--- a/g10/progress.c
+++ b/g10/progress.c
@@ -131,7 +131,7 @@ progress_filter (void *opaque, int control,
release_progress_context (pfx);
}
else if (control == IOBUFCTRL_DESC)
- *(char**)buf = "progress_filter";
+ strncpy (buf, "progress_filter", *ret_len);
return rc;
}
diff --git a/g10/textfilter.c b/g10/textfilter.c
index 394d9c3..dbef2af 100644
--- a/g10/textfilter.c
+++ b/g10/textfilter.c
@@ -150,7 +150,7 @@ text_filter( void *opaque, int control,
tfx->buffer = NULL;
}
else if( control == IOBUFCTRL_DESC )
- *(char**)buf = "text_filter";
+ strncpy (buf, "text_filter", *ret_len);
return rc;
}
--
More information about the Gnupg-devel
mailing list