On Tue, Feb 2, 2010 at 10:35 PM, Jeff Garzik j...@garzik.org wrote:
On 01/29/2010 08:01 PM, Colin McCabe wrote:
That seems reasonable. All of those functions could be looked at as
common.
The intention was to group together a bunch of functions that were
useful for packet formatting. Although the bulk of the formatting is
done by XDR, there are some things that XDR doesn't do, like
generating and checking SHA sums. cld_dump_buf and cld_pkt_hdr_to_str
are purely for debugging purposes, but they seemed like something that
would be generally useful for developers making packet format changes.
I know that they were useful to me.
I finally pushed out several changes split off from your main XDR patch, to
the upstream cld git repo. It took longer than expected because I would
make changes of my own along the way, which, each time, necessitated a
rediff+rebuild between current cld working tree and cld + xdr working
tree.
The attached patch is what remains to be split up and committed. I have
definitely whittled it down, and along the way, moved and renamed some
things on my own. With regards to cld_fmt.*, my main objection was to
creating too many to-be-installed headers. One more header can be a pain
for us and for users, while one more source file in cld/lib/ is no big deal.
Thus, the intention is to eliminate cld_fmt.h (newly renamed to cld_pkt.h)
altogether, while keeping the arrangement you created in cld_fmt.c (newly
renamed to lib/pkt.c).
Yeah, after I looked at the v2 patch I realized it wasn't separated
out well enough.
So I was working on a v3 patch series that had the following changes:
1. kill CLD_MAX_PKT_MSG and add CLD_MAX_PAYLOAD_SZ
2. reformat Makefile.am files (to one file per line for nicer diffs)
3. move cld_msg macros into cld_common
4. some style issues
5. add cld_fmt.c (now called cld_pkt.c)
6. create cldc_call_opts_get_data and switch to using it in test/ and tool/
7. the big XDR changes.
It looks like 2, 3, 4, 5 have already come to pass (thanks Jeff!)
I just sent out #1 and another patch that is related to CLD_INODE_NAME_MAX
I think I'll send out a patch for #6 soon.
I will continue whittling down the patch until it just contains the XDR
changes themselves.
In tools/Makefile.am, I don't think you need $(top_srcdir)/lib any
more, since cld_msg_rpc.h is now an installed header.
Colin
--
To unsubscribe from this list: send the line unsubscribe hail-devel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html