10:59 a.m.
Hi,
Here is one trivial initialization fix and another patch to convert a huge macro
to an inline function. The result of the expansion would show up in an assertion
which triggered a -Woverlength-strings warning.
Peter Wu (2):
Fix garbage return value on error
Fix overly long assertion string
lib/hivex-internal.h | 28 ++++++++++++++++------------
lib/node.c | 18 +++++++++---------
lib/utf16.c | 2 +-
lib/value.c | 14 +++++++-------
lib/write.c | 22 +++++++++++-----------
5 files changed, 44 insertions(+), 40 deletions(-)
--
2.0.2
10:59 a.m.
New subject: [PATCH 1/2] Fix garbage return value on error
If str contains invalid characters, ret is not initialised. Caught by
Clang static analyzer.
---
lib/utf16.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/utf16.c b/lib/utf16.c
index f4299f2..7d275fd 100644
--- a/lib/utf16.c
+++ b/lib/utf16.c
@@ -128,7 +128,7 @@ size_t
_hivex_utf8_strlen (const char* str, size_t len, int utf16)
{
const char *encoding = utf16 ? "UTF-16LE" : "LATIN1";
- size_t ret;
+ size_t ret = 0;
char *buf = _hivex_recode(encoding, str, len, "UTF-8", &ret);
free(buf);
return ret;
--
2.0.2
10:59 a.m.
New subject: [PATCH 2/2] Fix overly long assertion string
With GCC 4.9.1 I get a -Woverlength-strings warning:
hivex-internal.h:123:23: error: string length ‘3823’ is greater than ...
(STREQLEN (((struct ntreg_hbin_block *)((char *) (h)->addr + (offs)))->id,
(eqid), 2))
The compile command can be shortened to:
cd lib && cpp -DHAVE_CONFIG_H -I.. -I../gnulib/lib -I. \
-Woverlength-strings -Werror -g -O2 write.c
After that, be amazed at the huge assertion string produced by the
macro which is also visible in libhivex.so (two times).
To fix that mess, use an inline function. The `STR*` macros were moved
and a string.h header added since STREQLEN is used in the function.
---
lib/hivex-internal.h | 28 ++++++++++++++++------------
lib/node.c | 18 +++++++++---------
lib/value.c | 14 +++++++-------
lib/write.c | 22 +++++++++++-----------
4 files changed, 43 insertions(+), 39 deletions(-)
diff --git a/lib/hivex-internal.h b/lib/hivex-internal.h
index bfd24c8..e59084d 100644
--- a/lib/hivex-internal.h
+++ b/lib/hivex-internal.h
@@ -21,9 +21,20 @@
#include <stdarg.h>
#include <stddef.h>
+#include <string.h>
#include "byte_conversions.h"
+#define STREQ(a,b) (strcmp((a),(b)) == 0)
+#define STRCASEEQ(a,b) (strcasecmp((a),(b)) == 0)
+#define STRNEQ(a,b) (strcmp((a),(b)) != 0)
+#define STRCASENEQ(a,b) (strcasecmp((a),(b)) != 0)
+#define STREQLEN(a,b,n) (strncmp((a),(b),(n)) == 0)
+#define STRCASEEQLEN(a,b,n) (strncasecmp((a),(b),(n)) == 0)
+#define STRNEQLEN(a,b,n) (strncmp((a),(b),(n)) != 0)
+#define STRCASENEQLEN(a,b,n) (strncasecmp((a),(b),(n)) != 0)
+#define STRPREFIX(a,b) (strncmp((a),(b),strlen((b))) == 0)
+
struct hive_h {
char *filename;
int fd;
@@ -119,8 +130,11 @@ struct ntreg_hbin_block {
/* Block data follows here. */
} __attribute__((__packed__));
-#define BLOCK_ID_EQ(h,offs,eqid) \
- (STREQLEN (((struct ntreg_hbin_block *)((char *) (h)->addr + (offs)))->id,
(eqid), 2))
+static inline int
+block_id_eq (const hive_h *h, size_t offs, const char *eqid)
+{
+ return (STREQLEN (((struct ntreg_hbin_block *)((char *) (h)->addr + (offs)))->id,
(eqid), 2));
+}
struct ntreg_nk_record {
int32_t seg_len; /* length (always -ve because used) */
@@ -285,16 +299,6 @@ extern void _hivex_free_strings (char **argv);
/* value.c */
extern int _hivex_get_values (hive_h *h, hive_node_h node, hive_value_h **values_ret,
size_t **blocks_ret);
-#define STREQ(a,b) (strcmp((a),(b)) == 0)
-#define STRCASEEQ(a,b) (strcasecmp((a),(b)) == 0)
-#define STRNEQ(a,b) (strcmp((a),(b)) != 0)
-#define STRCASENEQ(a,b) (strcasecmp((a),(b)) != 0)
-#define STREQLEN(a,b,n) (strncmp((a),(b),(n)) == 0)
-#define STRCASEEQLEN(a,b,n) (strncasecmp((a),(b),(n)) == 0)
-#define STRNEQLEN(a,b,n) (strncmp((a),(b),(n)) != 0)
-#define STRCASENEQLEN(a,b,n) (strncasecmp((a),(b),(n)) != 0)
-#define STRPREFIX(a,b) (strncmp((a),(b),strlen((b))) == 0)
-
#define DEBUG(lvl,fs,...) \
do { \
if (h->msglvl >= (lvl)) { \
diff --git a/lib/node.c b/lib/node.c
index 22d1861..1fb48cf 100644
--- a/lib/node.c
+++ b/lib/node.c
@@ -49,7 +49,7 @@ hivex_root (hive_h *h)
size_t
hivex_node_struct_length (hive_h *h, hive_node_h node)
{
- if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ if (!IS_VALID_BLOCK (h, node) || !block_id_eq (h, node, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return 0;
}
@@ -71,7 +71,7 @@ hivex_node_struct_length (hive_h *h, hive_node_h node)
char *
hivex_node_name (hive_h *h, hive_node_h node)
{
- if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ if (!IS_VALID_BLOCK (h, node) || !block_id_eq (h, node, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return NULL;
}
@@ -99,7 +99,7 @@ hivex_node_name (hive_h *h, hive_node_h node)
size_t
hivex_node_name_len (hive_h *h, hive_node_h node)
{
- if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ if (!IS_VALID_BLOCK (h, node) || !block_id_eq (h, node, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return 0;
}
@@ -143,7 +143,7 @@ hivex_node_timestamp (hive_h *h, hive_node_h node)
{
int64_t ret;
- if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ if (!IS_VALID_BLOCK (h, node) || !block_id_eq (h, node, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return -1;
}
@@ -165,7 +165,7 @@ hivex_node_timestamp (hive_h *h, hive_node_h node)
hive_security_h
hivex_node_security (hive_h *h, hive_node_h node)
{
- if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ if (!IS_VALID_BLOCK (h, node) || !block_id_eq (h, node, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return 0;
}
@@ -184,7 +184,7 @@ hivex_node_security (hive_h *h, hive_node_h node)
hive_classname_h
hivex_node_classname (hive_h *h, hive_node_h node)
{
- if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ if (!IS_VALID_BLOCK (h, node) || !block_id_eq (h, node, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return 0;
}
@@ -283,7 +283,7 @@ _hivex_get_children (hive_h *h, hive_node_h node,
hive_node_h **children_ret, size_t **blocks_ret,
int flags)
{
- if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ if (!IS_VALID_BLOCK (h, node) || !block_id_eq (h, node, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return -1;
}
@@ -491,7 +491,7 @@ check_child_is_nk_block (hive_h *h, hive_node_h child, int flags)
struct ntreg_hbin_block *block =
(struct ntreg_hbin_block *) ((char *) h->addr + child);
- if (!BLOCK_ID_EQ (h, child, "nk")) {
+ if (!block_id_eq (h, child, "nk")) {
SET_ERRNO (EFAULT, "subkey is not an 'nk' block (0x%zx, %d, %d)",
child, block->id[0], block->id[1]);
return -1;
@@ -546,7 +546,7 @@ hivex_node_get_child (hive_h *h, hive_node_h node, const char *nname)
hive_node_h
hivex_node_parent (hive_h *h, hive_node_h node)
{
- if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ if (!IS_VALID_BLOCK (h, node) || !block_id_eq (h, node, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return 0;
}
diff --git a/lib/value.c b/lib/value.c
index f222b41..1c2356f 100644
--- a/lib/value.c
+++ b/lib/value.c
@@ -37,7 +37,7 @@ int
_hivex_get_values (hive_h *h, hive_node_h node,
hive_value_h **values_ret, size_t **blocks_ret)
{
- if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ if (!IS_VALID_BLOCK (h, node) || !block_id_eq (h, node, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return -1;
}
@@ -175,7 +175,7 @@ hivex_value_struct_length (hive_h *h, hive_value_h value)
size_t
hivex_value_key_len (hive_h *h, hive_value_h value)
{
- if (!IS_VALID_BLOCK (h, value) || !BLOCK_ID_EQ (h, value, "vk")) {
+ if (!IS_VALID_BLOCK (h, value) || !block_id_eq (h, value, "vk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'vk' block");
return 0;
}
@@ -199,7 +199,7 @@ hivex_value_key_len (hive_h *h, hive_value_h value)
char *
hivex_value_key (hive_h *h, hive_value_h value)
{
- if (!IS_VALID_BLOCK (h, value) || !BLOCK_ID_EQ (h, value, "vk")) {
+ if (!IS_VALID_BLOCK (h, value) || !block_id_eq (h, value, "vk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'vk' block");
return 0;
}
@@ -225,7 +225,7 @@ hivex_value_key (hive_h *h, hive_value_h value)
int
hivex_value_type (hive_h *h, hive_value_h value, hive_type *t, size_t *len)
{
- if (!IS_VALID_BLOCK (h, value) || !BLOCK_ID_EQ (h, value, "vk")) {
+ if (!IS_VALID_BLOCK (h, value) || !block_id_eq (h, value, "vk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'vk' block");
return -1;
}
@@ -247,7 +247,7 @@ hivex_value_type (hive_h *h, hive_value_h value, hive_type *t, size_t
*len)
hive_value_h
hivex_value_data_cell_offset (hive_h *h, hive_value_h value, size_t *len)
{
- if (!IS_VALID_BLOCK (h, value) || !BLOCK_ID_EQ (h, value, "vk")) {
+ if (!IS_VALID_BLOCK (h, value) || !block_id_eq (h, value, "vk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'vk' block");
return 0;
}
@@ -300,7 +300,7 @@ char *
hivex_value_value (hive_h *h, hive_value_h value,
hive_type *t_rtn, size_t *len_rtn)
{
- if (!IS_VALID_BLOCK (h, value) || !BLOCK_ID_EQ (h, value, "vk")) {
+ if (!IS_VALID_BLOCK (h, value) || !block_id_eq (h, value, "vk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'vk' block");
return NULL;
}
@@ -368,7 +368,7 @@ hivex_value_value (hive_h *h, hive_value_h value,
return ret;
} else {
if (!IS_VALID_BLOCK (h, data_offset) ||
- !BLOCK_ID_EQ (h, data_offset, "db")) {
+ !block_id_eq (h, data_offset, "db")) {
SET_ERRNO (EINVAL,
"declared data length is longer than the block and "
"block is not a db block (data 0x%zx, data len %zu)",
diff --git a/lib/write.c b/lib/write.c
index 0b25549..9de8ecd 100644
--- a/lib/write.c
+++ b/lib/write.c
@@ -340,7 +340,7 @@ insert_lf_record (hive_h *h, size_t old_offs, size_t posn,
/* Work around C stupidity.
* http://www.redhat.com/archives/libguestfs/2010-February/msg00056.html
*/
- int test = BLOCK_ID_EQ (h, old_offs, "lf") || BLOCK_ID_EQ (h, old_offs,
"lh");
+ int test = block_id_eq (h, old_offs, "lf") || block_id_eq (h, old_offs,
"lh");
assert (test);
struct ntreg_lf_record *old_lf =
@@ -399,7 +399,7 @@ insert_li_record (hive_h *h, size_t old_offs, size_t posn,
const char *name, hive_node_h node)
{
assert (IS_VALID_BLOCK (h, old_offs));
- assert (BLOCK_ID_EQ (h, old_offs, "li"));
+ assert (block_id_eq (h, old_offs, "li"));
struct ntreg_ri_record *old_li =
(struct ntreg_ri_record *) ((char *) h->addr + old_offs);
@@ -452,7 +452,7 @@ static int
compare_name_with_nk_name (hive_h *h, const char *name, hive_node_h nk_offs)
{
assert (IS_VALID_BLOCK (h, nk_offs));
- assert (BLOCK_ID_EQ (h, nk_offs, "nk"));
+ assert (block_id_eq (h, nk_offs, "nk"));
/* Name in nk is not necessarily nul-terminated. */
char *nname = hivex_node_name (h, nk_offs);
@@ -493,7 +493,7 @@ insert_subkey (hive_h *h, const char *name,
* the end.
*/
for (i = 0; blocks[i] != 0; ++i) {
- if (BLOCK_ID_EQ (h, blocks[i], "lf") || BLOCK_ID_EQ (h, blocks[i],
"lh")) {
+ if (block_id_eq (h, blocks[i], "lf") || block_id_eq (h, blocks[i],
"lh")) {
old_offs = blocks[i];
old_lf = (struct ntreg_lf_record *) ((char *) h->addr + old_offs);
old_li = NULL;
@@ -504,7 +504,7 @@ insert_subkey (hive_h *h, const char *name,
goto insert_it;
}
}
- else if (BLOCK_ID_EQ (h, blocks[i], "li")) {
+ else if (block_id_eq (h, blocks[i], "li")) {
old_offs = blocks[i];
old_lf = NULL;
old_li = (struct ntreg_ri_record *) ((char *) h->addr + old_offs);
@@ -565,7 +565,7 @@ insert_subkey (hive_h *h, const char *name,
}
else {
for (i = 0; blocks[i] != 0; ++i) {
- if (BLOCK_ID_EQ (h, blocks[i], "ri")) {
+ if (block_id_eq (h, blocks[i], "ri")) {
struct ntreg_ri_record *ri =
(struct ntreg_ri_record *) ((char *) h->addr + blocks[i]);
for (j = 0; j < le16toh (ri->nr_offsets); ++j)
@@ -593,7 +593,7 @@ hivex_node_add_child (hive_h *h, hive_node_h parent, const char
*name)
{
CHECK_WRITABLE (0);
- if (!IS_VALID_BLOCK (h, parent) || !BLOCK_ID_EQ (h, parent, "nk")) {
+ if (!IS_VALID_BLOCK (h, parent) || !block_id_eq (h, parent, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return 0;
}
@@ -649,7 +649,7 @@ hivex_node_add_child (hive_h *h, hive_node_h parent, const char
*name)
size_t parent_sk_offset = le32toh (parent_nk->sk);
parent_sk_offset += 0x1000;
if (!IS_VALID_BLOCK (h, parent_sk_offset) ||
- !BLOCK_ID_EQ (h, parent_sk_offset, "sk")) {
+ !block_id_eq (h, parent_sk_offset, "sk")) {
SET_ERRNO (EFAULT,
"parent sk is not a valid block (%zu)", parent_sk_offset);
return 0;
@@ -747,7 +747,7 @@ hivex_node_add_child (hive_h *h, hive_node_h parent, const char
*name)
static int
delete_sk (hive_h *h, size_t sk_offset)
{
- if (!IS_VALID_BLOCK (h, sk_offset) || !BLOCK_ID_EQ (h, sk_offset, "sk")) {
+ if (!IS_VALID_BLOCK (h, sk_offset) || !block_id_eq (h, sk_offset, "sk")) {
SET_ERRNO (EFAULT, "not an sk record: 0x%zx", sk_offset);
return -1;
}
@@ -852,7 +852,7 @@ hivex_node_delete_child (hive_h *h, hive_node_h node)
{
CHECK_WRITABLE (-1);
- if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ if (!IS_VALID_BLOCK (h, node) || !block_id_eq (h, node, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return -1;
}
@@ -925,7 +925,7 @@ hivex_node_set_values (hive_h *h, hive_node_h node,
{
CHECK_WRITABLE (-1);
- if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ if (!IS_VALID_BLOCK (h, node) || !block_id_eq (h, node, "nk")) {
SET_ERRNO (EINVAL, "invalid block or not an 'nk' block");
return -1;
}
--
2.0.2
11:12 a.m.
On Thu, Aug 07, 2014 at 05:59:48PM +0200, Peter Wu wrote:
Hi,
Here is one trivial initialization fix and another patch to convert a huge macro
to an inline function. The result of the expansion would show up in an assertion
which triggered a -Woverlength-strings warning.
Thanks -- I have pushed both of these patches upstream.
Did you come to a final conclusion about the Python binding problem?
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
11:16 a.m.
Hi Richard,
Thanks for pushing. I have not yet had the time to continue on that, might have a look at
it in the night. These patches were sitting in my tree so I thought it would be a good
idea to send it early.
Kind regards,
Peter
https://lekensteyn.nl
(pardon my brevity, top-posting and formatting, sent from my phone)
On August 7, 2014 6:12:55 PM CEST, "Richard W.M. Jones"
<rjones(a)redhat.com> wrote:
On Thu, Aug 07, 2014 at 05:59:48PM +0200, Peter Wu wrote:
> Hi,
>
> Here is one trivial initialization fix and another patch to convert a
huge macro
> to an inline function. The result of the expansion would show up in
an assertion
> which triggered a -Woverlength-strings warning.
Thanks -- I have pushed both of these patches upstream.
Did you come to a final conclusion about the Python binding problem?
Rich.
3684
days inactive
3684
days old
4 comments
2 participants
participants (2)
-
Peter Wu -
Richard W.M. Jones