[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 2/3] vmport: Add VMware provided include files.



On 09/02/14 03:34, Jan Beulich wrote:
On 01.09.14 at 17:33, <dslutz@xxxxxxxxxxx> wrote:
These 2 files: backdoor_def.h and guest_msg_def.h come from:

http://packages.vmware.com/tools/esx/3.5latest/rhel4/SRPMS/index.html
  open-vm-tools-kmod-7.4.8-396269.423167.src.rpm
   open-vm-tools-kmod-7.4.8.tar.gz
    vmhgfs/backdoor_def.h
    vmhgfs/guest_msg_def.h

and are unchanged.
Okay, I can the value in keeping them as is even if they're in
conflict with some of our coding style rules. But ...

--- /dev/null
+++ b/xen/arch/x86/hvm/vmport/backdoor_def.h
@@ -0,0 +1,167 @@
+/* **********************************************************
+ * Copyright 1998 VMware, Inc.  All rights reserved.
+ * **********************************************************
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation version 2 and no later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
+ */
+
+/*
+ * backdoor_def.h --
+ *
+ * This contains backdoor defines that can be included from
+ * an assembly language file.
+ */
+
+
+
+#ifndef _BACKDOOR_DEF_H_
+#define _BACKDOOR_DEF_H_
... these guards have the (theoretical at this point) risk of clashing
(apart from violating the C standard's naming rules) and ...


Yes. That is why I had the comment:


/* Note: VMPORT_PORT and VMPORT_MAGIC is also defined as BDOOR_PORT
* and BDOOR_MAGIC in backdoor_def.h Defined in vmport.h also.
*/

because I did not want the reset of xen to be including this file.

If I have found the right "C standard's naming rule", this file is not
alone in violating it:

The following is the C standard's specification for naming the identifiers (C11 draft <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf>):

*7.1.3 Reserved identifiers*

Each header declares or defines all identifiers listed in its associated subclause, and optionally declares or defines identifiers listed in its associated future library directions subclause and identifiers which are always reserved either for any use or for use as file scope identifiers.

— All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.



For example:


xen/arch/x86/boot/video.h:#ifndef __BOOT_VIDEO_H__
xen/arch/arm/vtimer.h:#ifndef __ARCH_ARM_VTIMER_H__
xen/arch/x86/cpu/mcheck/barrier.h:#ifndef _MCHECK_BARRIER_H
tools/xenstore/talloc.h:#ifndef _TALLOC_H_




+#define INCLUDE_ALLOW_MODULE
+#define INCLUDE_ALLOW_USERLEVEL
+#define INCLUDE_ALLOW_VMMEXT
+#define INCLUDE_ALLOW_VMCORE
+#define INCLUDE_ALLOW_VMKERNEL
+#include "includeCheck.h"
... the patch is obviously incomplete without this header, the name
of which is certainly not in line with our way of naming files.

Yes. And an empty file of that name is added in the next patch.


So I am getting the feeling is that it would be better to have a patch
adjusting these include files to better fit in xen, and then move them
out of the new xen/arch/x86/hvm/vmware directory.

In any case I would continue with this patch to provide the history of
where the include file came from.

-Don Slutz
Jan



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.