Re: [Xen-devel] [PATCH 21/21] libxl: DO NOT APPLY enforce prohibition on internal

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [PATCH 21/21] libxl: DO NOT APPLY enforce prohibition 
on internal"):
> On Fri, 2012-06-15 at 12:54 +0100, Ian Jackson wrote:
> > DO NOT APPLY THIS PATCH.
> > It contains -Wno-error.  Without that it would break the build.
> 
> I'm happy with the shape of this patch, and apart from the above caveat
> I'd be happy to Ack it, assuming it were preceded by fixes to those
> issues.
> 
> I fear slightly that we will be continually forgetting to add the tag
> during review. Is there a way we could augment with run time checking?
> Perhaps LIBXL__INIT_EGC can tell if a CTX is not "outer" somehow?

No, I think it can't without doing a lot of very tiresome things at
runtime with thread-local variables.

It would be possible via a tangle of #defines to arrange that the
_definition_ of such a function would have to have a weird type:

So you would do:

   /* libxl.h */

   int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
                               libxl_device_disk *disk,
                               const libxl_asyncop_how *ao_how)
                               LIBXL_EXTERNAL_CALLERS_ONLY;

   /* libxl.c */

   int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
                                libxl_device_disk *disk,
                                MAGIC_SPEEDBUMP_MACRO_TYPE *ao_how)
   {

This would stop you from accidentally writing an entirely new function
which took an ao_how, and just c&p'ing its declaration into libxl.h.
But there would still be nothing stopping you simply forgetting to add
the LIBXL_EXTERNAL_CALLERS_ONLY to the declaration.  That doesn't seem
worthwhile.


The only other thing would be to write a small Perl script to check
for obvious abuses.

[a bit later]

How about this:

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index c238ac0..1c8b62b 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -74,7 +74,8 @@ LIBXL_OBJS += _libxl_types.o libxl_flask.o 
_libxl_types_internal.o
 $(LIBXL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) 
$(CFLAGS_libxenstore) $(CFLAGS_libblktapctl) -include $(XEN_ROOT)/tools/config.h
 
 AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h _paths.h \
-       _libxl_save_msgs_callout.h _libxl_save_msgs_helper.h
+       _libxl_save_msgs_callout.h _libxl_save_msgs_helper.h \
+        libxl.api-ok
 AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c
 AUTOSRCS += _libxl_save_msgs_callout.c _libxl_save_msgs_helper.c
 LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_l.o libxlu_cfg.o \
@@ -113,6 +114,15 @@ $(LIBXL_OBJS) $(LIBXLU_OBJS) $(XL_OBJS): $(AUTOINCS)
 genpath-target = $(call buildmakevars2file,_paths.h.tmp)
 $(eval $(genpath-target))
 
+libxl.api-ok: check-libxl-api-rules libxl.api-for-check
+       perl $^
+
+%.api-for-check: %.h
+       $(CC) $(CPPFLAGS) $(CFLAGS) $(CFLAGS_$*.o) -c -E $< $(APPEND_CFLAGS) \
+               -DLIBXL_EXTERNAL_CALLERS_ONLY=LIBXL_EXTERNAL_CALLERS_ONLY \
+               >$@.new
+       $(call move-if-changed,$@.new,$@)
+
 _paths.h: genpath
        sed -e "s/\([^=]*\)=\(.*\)/#define \1 \2/g" $@.tmp >$@.2.tmp
        rm -f $@.tmp
@@ -200,7 +210,7 @@ install: all
 .PHONY: clean
 clean:
        $(RM) -f _*.h *.o *.so* *.a $(CLIENTS) $(DEPS)
-       $(RM) -f _*.c *.pyc _paths.*.tmp
+       $(RM) -f _*.c *.pyc _paths.*.tmp *.api-for-check
        $(RM) -f testidl.c.new testidl.c
 #      $(RM) -f $(AUTOSRCS) $(AUTOINCS)
 
diff --git a/tools/libxl/check-libxl-api-rules 
b/tools/libxl/check-libxl-api-rules
new file mode 100755
index 0000000..e056573
--- /dev/null
+++ b/tools/libxl/check-libxl-api-rules
@@ -0,0 +1,15 @@
+#!/usr/bin/perl -w
+use strict;
+our $needed=0;
+while (<>) {
+      if (m/libxl_asyncop_how[^;]/) {
+         $needed=1;
+      }      
+      if (m/LIBXL_EXTERNAL_CALLERS_ONLY/) {
+          $needed=0;
+      }
+      next unless $needed;
+      if (m/\;/) {
+          die "$ARGV:$.:missing LIBXL_EXTERNAL_CALLERS_ONLY";
+      }
+}

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel