On 7/20/07, Paul Clements <[email protected]> wrote:
Denis Cheng wrote:
> this memcpy looks so strange, in fact it's merely a pointer dereference,
> so I change the parameter's type to refer it more directly,
> this could make the memcpy not needed anymore.
>
> in the function nbd_read_stat where nbd_find_request is only once called,
> the parameter served should be transformed accordingly.
This is really a matter of preference. The generated code ends up being
about the same, I think, while your patch makes the call to
nbd_find_request kind of obtuse. Also, the memcpy's are balanced between
send_req and find_request, so you can quickly see how the data is being
transferred (from req into handle, and then back again). Your patch
makes this less clear, at least to me.
With one explicit memcpy stripped out, I think it's more clear to
nbd_find_request.
In nbd_read_stat, the cast to (struct request **) is not apparent, I
must admit; but I think the best solution is declaring other few
structs to make it clear, it's due to the lack of description of nbd
client and server communication protocol.
BTW, I think the nbd driver needs a clear documentation (its main site
http://nbd.sourceforge.net/ does not give it):
1. When nbd_find_request is needed to call, the 8 byte memory of char
handle[8] field in struct nbd_reply actually stores a pointer (struct
request *), that pointer is received from the network. Since a pointer
is only meaningful to the host, transfering it over the network will
be unreliable, I don't think it's a good design,
--
Denis Cheng
Linux Application Developer
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
