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

Reply via email to