Bardur Arantsson wrote:
Patrice Dumas wrote:
[--snip--]
(I'm going to split my reply up because some of this could go on for a
while... I'll probably split the reply up over a couple of days as well.)
The macro KEY_METAINFO_SIZE also has issues on my AMD64:
#define KEY_METAINFO_SIZE(k) ((unsigned int)&(k->recordSize) -
unsigned int)k)
is incorrect when the result is used as a "size_t" in various locations.
The correct definition is
#define KEY_METAINFO_SIZE(k) ((size_t)&(k->recordSize) - (size_t)k)
Since recordSize is size_t in _Key, wouldn't it be even better to
use
#define KEY_METAINFO_SIZE(k) ((k->recordSize) - (size_t)k)
Yup, I hadn't checked the definition of recordSize.
Reinvestigating this, the proper definition is as follows:
#define KEY_METAINFO_SIZE(k) ((size_t) (((char *) (&k->recordSize))
- (char *) k)))
Note that the original macro is actually using the *address* of the
recordSize member and not the value! Instead of casting the addresses
directly to (size_t), however, I've added an intermediate cast to (char
*). This is necessary because a size_t may not be able to hold an
arbitrary address -- I don't recall the precise guarantees for
sizeof(size_t) vs. sizeof(any *), but I guess it's better to be safe
than sorry. We *can* be sure that the computed difference is less than
whan can be represented in a size_t, though, so the final cast to
(size_t) cannot result in truncation.
I've attached a patch which should be applied (I don't have commit
access) and which silences the gcc warnings on AMD64.
All this is of course an ugly hack which is only necessary because of
what I can only describe as the struct _Key's severely flawed design.
The _Key struct should in fact actually consist of two sub-structs, thus:
struct _KeyMetaData {
// declare all the metadata fields:
uint8_t type;
uid_t uid;
uid_t gid;
mode_t access;
time_t atime;
time_t mtime;
time_t ctime;
size_t commentSize;
size_t dataSize;
};
struct _KeyData {
uint32_t flags;
char * key;
char * comment;
char * userDomain;
void * data; /**< The value, which is a NULL terminated
string or binary */
struct _Key * next; /**< Link to the next object in a KeySet
context */
}
struct _Key {
_KeyMetaData metadata;
_KeyData data;
}
which would make things considerably cleaner all round and obviate such
bizarre hacks as KEY_METAINFO_SIZE. But I guess we're too far along to
change this. What do the Powers That Be think about such a change?
--
Bardur Arantsson
<[EMAIL PROTECTED]>
- Sometimes I think Trent just needs a cup of hot chocolate and a
blankie.
Tori Amos commenting on Nine Inch Nails
Index: src/include/kdbprivate.h
===================================================================
--- src/include/kdbprivate.h (revision 860)
+++ src/include/kdbprivate.h (working copy)
@@ -183,7 +183,7 @@
* of your backend.
* @ingroup backend
*/
-#define KEY_METAINFO_SIZE(k) ((unsigned int)&(k->recordSize) - (unsigned int)k)
+#define KEY_METAINFO_SIZE(k) ((size_t) (((char *) (&k->recordSize)) - ((char
*) k)))
Index: src/backends/filesys/filesys.c
===================================================================
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Registry-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/registry-list