Re: updated orangefs tree at kernel.org

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



May I politely inquire about the intended meaning of the following
definition?
#define MAX_ALIGNED_DEV_REQ_DOWNSIZE                            \
                (MAX_DEV_REQ_DOWNSIZE +                         \
                        ((((MAX_DEV_REQ_DOWNSIZE /              \
                                (BITS_PER_LONG_DIV_8)) *        \
                                (BITS_PER_LONG_DIV_8)) +        \
                            (BITS_PER_LONG_DIV_8)) -            \
                        MAX_DEV_REQ_DOWNSIZE))

1) (x + (y - x)) is more commonly spelled as y
2) ((x / y) * y + y) might not be doing what you think it is doing
3) aligning the thing to multiple of sizeof(long) is an odd thing
to do, seeing that the value being aligned is 16 + sizeof
of a structure that contains pointers (which is an atrocity in its
own right, BTW).
4) on the related note, what is the meaning of
        static int max_downsize = MAX_ALIGNED_DEV_REQ_DOWNSIZE;
        int ret = 0, num_remaining = max_downsize;
(with no code other code touching that max_downsize thing)?
5) having collected the contents of the first 4 segments of your writev()
argument and having verified that their total size does not exceed
MAX_ALIGNED_DEV_REQ_DOWNSIZE, you proceed to do this:
                payload_size -= (2 * sizeof(__s32) + sizeof(__u64));
                if (payload_size <= sizeof(struct pvfs2_downcall_s))
                        /* copy the passed in downcall into the op */
                        memcpy(&op->downcall,
                               ptr,
                               sizeof(struct pvfs2_downcall_s));
                else
                        gossip_debug(GOSSIP_DEV_DEBUG,
                                     "writev: Ignoring %d bytes\n",
                                     payload_size);
upon the entry payload_size contains the amount of data collected.  What is the
intended meaning of the 'else' branch in there?  Note that it *is* possible
to hit, exactly because MAX_ALIGNED_DEV_REQ_DOWNSIZE is sizeof(long)
greater than 16 + sizeof(struct pvfs2_downcall_s).  What should happen if we
do hit it?  I do understand the "we shall whine" part, but then what?  Leave
the op->downcall uninitialized and proceed with whatever might've been there?

That code does marshalling of userland-supplied data.  Worse, the ABI is both
undocumented and utterly... I believe "idiosyncratic" would be a printable
term for that kind of thing.  Sigh...
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux