Hi all, I'm currently reworking this patch to address the concerns mentioned here (mostly autotools-related). I'm also going to separate it into multiple patches. A few questions: I would really like cld_msg_rpc.h to show up in "include," for consistency with the other headers. Is it possible for cld_msg_rpc.x to live in lib/ and export the header into the include/ directory? I've been reading everything I can about automake, but I still haven't been able to figure out if this kind of cross-directory dependency between two directories at the same depth is possible. I really don't want to put cld_msg_rpc_xdr.c in include/. But so far I can't figure out how to have the header there, without the .c file... Colin On Wed, Jan 20, 2010 at 6:38 PM, Colin McCabe <cmccabe@xxxxxxxxxxxxxx> wrote: > On Wed, Jan 20, 2010 at 5:38 PM, Jeff Garzik <jeff@xxxxxxxxxx> wrote: >> On 01/15/2010 12:49 PM, Colin McCabe wrote: >>> >>> This patch moves CLD from using manual data serialization to using XDR >>> (via >>> rpcgen). Both the packet header and the message body are now serialized >>> and >>> deserialized using XDR. This makes it easy to have a variable-length >>> packet >>> header, as well as a variable-length message body. >>> >>> This patch introduces a minor libcldc API change in struct cldc_call_opts. >>> >>> v2 changes: >>> * Added __cld prefix to functions in cld_fmt.c >>> >>> * When decoding CMT_GET messages, we should store the payload in the >>> session structure, rather than in cldc_call_opts. >>> >>> * Got rid of pkt_is_first, pkt_is_last, in favor of a flags-based >>> approach. >>> >>> * Killed CLD_MAX_PKT_MSG. It's more efficient to only allocate as much >>> space >>> as you need, rather than always allocating space for 128 packets in a >>> message. >>> >>> * Created CLD_MAX_PAYLOAD_SZ to mean the maximum size of the data that can >>> be >>> sent with GET or PUT. This is different (and smaller than!) the maximum >>> message size. >>> >>> * automake: Add cld_msg_rpc.h to BUILT_SOURCES >>> >>> * automake: Add lib dir to INCLUDES >>> >>> * automake: "make clean" now deletes XDR build products >>> >>> Signed-off-by: Colin McCabe<cmccabe@xxxxxxxxxxxxxx> >>> --- >>> .gitignore | 4 + >>> include/Makefile.am | 2 +- >>> include/cld_common.h | 10 + >>> include/cld_fmt.h | 89 +++++ >>> include/cld_msg.h | 252 ------------- >>> include/cldc.h | 36 +- >>> lib/Makefile.am | 19 +- >>> lib/cld_fmt.c | 193 ++++++++++ >>> lib/cld_msg_rpc.x | 218 +++++++++++ >>> lib/cldc.c | 966 >>> +++++++++++++++++++++--------------------------- >>> lib/common.c | 6 +- >>> server/Makefile.am | 9 +- >>> server/cld.h | 97 ++++-- >>> server/cldb.h | 2 +- >>> server/msg.c | 304 ++++++---------- >>> server/server.c | 743 ++++++++++++++++++++----------------- >>> server/session.c | 396 ++++++++++----------- >>> test/Makefile.am | 5 +- >>> test/load-file-event.c | 14 +- >>> test/save-file-event.c | 2 - >>> tools/cldcli.c | 22 +- >>> 21 files changed, 1777 insertions(+), 1612 deletions(-) >>> create mode 100644 include/cld_fmt.h >>> delete mode 100644 include/cld_msg.h >>> create mode 100644 lib/cld_fmt.c >>> create mode 100644 lib/cld_msg_rpc.x >> >> Looking good... we are definitely getting closer. >> >> Currently, it still does not pass "make distcheck", which presages a lot of >> installation problems. Notably, >> #include <lib/cld_msg_rpc.h> >> is not consistent with the rest of the headers, producing things such as >> "make distcheck" build breakage: > > I guess distcheck is pretty important to run when making a change like > this. For some reason I forgot to try that. :( > >> >>> libtool: compile: gcc -DHAVE_CONFIG_H -I. -I../../lib -I.. >>> -I../../include -pthread -I/usr/include/glib-2.0 >>> -I/usr/lib64/glib-2.0/include -g -O2 -MT cldc.lo -MD -MP -MF .deps/cldc.Tpo >>> -c ../../lib/cldc.c -fPIC -DPIC -o .libs/cldc.o >>> In file included from ../../lib/cldc.c:37: >>> ../../include/cldc.h:26:29: error: lib/cld_msg_rpc.h: No such file or >>> directory >> >> You will either need to move the header to $cld/include/ or update INCLUDES >> in various */Makefile.am files to reference $(top_srcdir)/lib (which I see >> you actually did, in a few cases). >> >> Also, it would be nice if this did not introduce warnings. A big point with >> the testsuite, and minor point with compiler warnings, is that -- more than >> anything else -- we do not want to introduce regressions. > > When I did "make all" with -Wall, the only warnings I saw were from > the generated xdr serialization files. I'd like to figure out how to > suppress those (we really should somehow tack -Wnone onto the CFLAGs > of machine-generated files), but I haven't figured out how to do that > yet. > > I was thinking of doing it in a follow-on change, since this change is > already pretty big. > > 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
