On Tue, Feb 2, 2010 at 10:35 PM, Jeff Garzik <jeff@xxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
