libdrm: Patch to compile on hurd.

Submitted by Tobias Frost on April 21, 2016, 5:30 a.m.

Details

Message ID 1461216633.14744.4.camel@frost.de
State New
Headers show
Series "libdrm: Patch to compile on hurd." ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Tobias Frost April 21, 2016, 5:30 a.m.
Hallo,

attached is a patch that makes libdrm compile on hurd.

(Note: I intentionally said compile, as I have no way to see if it
actually works there.)

The patch is created in a way to be neutral on all other archs;
it is mostly about PATH_MAX, which does not exist on that arch.

Maybe you find the patch useful.


Thanks!

--

Patch hide | download patch | download mbox

--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -42,10 +42,19 @@ 
 #include <asm/ioctl.h>
 typedef unsigned int drm_handle_t;
 
+#elif defined(__gnu_hurd__)
+#include <stdint.h>
+#include <mach/i386/ioccom.h>
+
 #else /* One of the BSDs */
 
 #include <sys/ioccom.h>
 #include <sys/types.h>
+
+#endif
+
+#if !defined(__linux__)
+
 typedef int8_t   __s8;
 typedef uint8_t  __u8;
 typedef int16_t  __s16;
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -58,7 +58,11 @@ 
 
 #else /* One of the *BSDs */
 
+#if defined(__gnu_hurd__)
+#include <mach/i386/ioccom.h>
+#else
 #include <sys/ioccom.h>
+#endif
 #define DRM_IOCTL_NR(n)         ((n) & 0xff)
 #define DRM_IOC_VOID            IOC_VOID
 #define DRM_IOC_READ            IOC_OUT
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -103,6 +103,16 @@ 
 
 #define memclear(s) memset(&s, 0, sizeof(s))
 
+/* for systems like hurd which does not have PATH_MAX.
+ Usage is only for string manipulation, so it is save to define it.
+ 1kB will be plenty space...*/
+#ifndef PATH_MAX
+#define MY_PATH_MAX (1024)
+#else
+#define MY_PATH_MAX PATH_MAX
+#endif
+
+
 static drmServerInfoPtr drm_server_info;
 
 void drmSetServerInfo(drmServerInfoPtr info)
@@ -2835,14 +2845,15 @@ 
 static int drmParseSubsystemType(int maj, int min)
 {
 #ifdef __linux__
-    char path[PATH_MAX + 1];
-    char link[PATH_MAX + 1] = "";
+
+    char path[MY_PATH_MAX + 1];
+    char link[MY_PATH_MAX + 1] = "";
     char *name;
 
-    snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/subsystem",
+    snprintf(path, MY_PATH_MAX, "/sys/dev/char/%d:%d/device/subsystem",
              maj, min);
 
-    if (readlink(path, link, PATH_MAX) < 0)
+    if (readlink(path, link, MY_PATH_MAX) < 0)
         return -errno;
 
     name = strrchr(link, '/');
@@ -2857,18 +2868,19 @@ 
 #warning "Missing implementation of drmParseSubsystemType"
     return -EINVAL;
 #endif
+
 }
 
 static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
 {
 #ifdef __linux__
-    char path[PATH_MAX + 1];
+    char path[MY_PATH_MAX + 1];
     char data[128 + 1];
     char *str;
     int domain, bus, dev, func;
     int fd, ret;
 
-    snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/uevent", maj, min);
+    snprintf(path, MY_PATH_MAX, "/sys/dev/char/%d:%d/device/uevent", maj, min);
     fd = open(path, O_RDONLY);
     if (fd < 0)
         return -errno;
@@ -2949,11 +2961,11 @@ 
                                  drmPciDeviceInfoPtr device)
 {
 #ifdef __linux__
-    char path[PATH_MAX + 1];
+    char path[MY_PATH_MAX + 1];
     unsigned char config[64];
     int fd, ret;
 
-    snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config", d_name);
+    snprintf(path, MY_PATH_MAX, "/sys/class/drm/%s/device/config", d_name);
     fd = open(path, O_RDONLY);
     if (fd < 0)
         return -errno;
@@ -3082,7 +3094,7 @@ 
     DIR *sysdir;
     struct dirent *dent;
     struct stat sbuf;
-    char node[PATH_MAX + 1];
+    char node[MY_PATH_MAX + 1];
     int node_type, subsystem_type;
     int maj, min;
     int ret, i, node_count;
@@ -3118,7 +3130,7 @@ 
         if (node_type < 0)
             continue;
 
-        snprintf(node, PATH_MAX, "%s/%s", DRM_DIR_NAME, dent->d_name);
+        snprintf(node, MY_PATH_MAX, "%s/%s", DRM_DIR_NAME, dent->d_name);
         if (stat(node, &sbuf))
             continue;
 
@@ -3198,7 +3210,7 @@ 
     DIR *sysdir;
     struct dirent *dent;
     struct stat sbuf;
-    char node[PATH_MAX + 1];
+    char node[MY_PATH_MAX + 1];
     int node_type, subsystem_type;
     int maj, min;
     int ret, i, node_count, device_count;
@@ -3220,7 +3232,7 @@ 
         if (node_type < 0)
             continue;
 
-        snprintf(node, PATH_MAX, "%s/%s", DRM_DIR_NAME, dent->d_name);
+        snprintf(node, MY_PATH_MAX, "%s/%s", DRM_DIR_NAME, dent->d_name);
         if (stat(node, &sbuf))
             continue;
 

Comments

Hi Tobias,

On 21 April 2016 at 06:30, Tobias Frost <tobi@frost.de> wrote:
> Hallo,
>
> attached is a patch that makes libdrm compile on hurd.
>
Thanks for the patch.

> (Note: I intentionally said compile, as I have no way to see if it
> actually works there.)
>
Step one is actually making it build and step two involves testing it ;-)
I've CC'ed Gary Wong, a gnu developer who has sent patches about mesa
in the past.

> The patch is created in a way to be neutral on all other archs;
> it is mostly about PATH_MAX, which does not exist on that arch.
>
> Maybe you find the patch useful.
>
Sure do, just a few suggestions.

Can you please split out the include/drm/* change to a separate patch
and base it against the kernel UAPI headers.

On the PATH_MAX front, please drop the whitespace change, and define
PATH_MAX at top level, rather than using MY_PATH_MAX throughout the
code.

Please CC me on the updated patches and please send them via git send-email.

Thanks
Emil
Hi Emil,


Am Donnerstag, den 21.04.2016, 13:33 +0100 schrieb Emil Velikov:
> Hi Tobias,
> 
> On 21 April 2016 at 06:30, Tobias Frost <tobi@frost.de> wrote:
> > Hallo,
> > 
> > attached is a patch that makes libdrm compile on hurd.
> > 
> Thanks for the patch.
> 
> > (Note: I intentionally said compile, as I have no way to see if it
> > actually works there.)
> > 
> Step one is actually making it build and step two involves testing it ;-)
> I've CC'ed Gary Wong, a gnu developer who has sent patches about mesa
> in the past.
> 
> > The patch is created in a way to be neutral on all other archs;
> > it is mostly about PATH_MAX, which does not exist on that arch.
> > 
> > Maybe you find the patch useful.
> > 
> Sure do, just a few suggestions.
> 
> Can you please split out the include/drm/* change to a separate patch
> and base it against the kernel UAPI headers.

I might need more instructions about this.. I'm unsure which repository
I should use as base... 

> On the PATH_MAX front, please drop the whitespace change, and define
> PATH_MAX at top level, rather than using MY_PATH_MAX throughout the
> code.

> Please CC me on the updated patches and please send them via git
> send-email.

On the way...
(the include/drm/* stuff is against trunk of the git repo, in the hope
that this is what you need -- see above)

> Thanks
> Emil

-- 
tobi
GITo [PATCH] On hurd other includes are required to compile.
Hi Tobias,

A couple of suggestions I missed the first time around.

On 24 April 2016 at 08:57, Tobias Frost <tobi@coldtobi.de> wrote:
> It is safe to define it here, as the code is only using it for string
> manipulation and not for syscalls that might make different assumptions.
> ---
>  xf86drm.c | 7 +++++++
>  xf86drm.h | 4 ++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 45aa5fc..4dac7a4 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -103,6 +103,13 @@
>
>  #define memclear(s) memset(&s, 0, sizeof(s))
>
> +/* for systems like hurd, which does not have PATH_MAX.
> + Usage is only for string manipulation, so it is save to define it.
> + 1kB will be plenty space.*/
> +#ifndef PATH_MAX
> +#define PATH_MAX (1024)
The brackets do not seem necessary.

> +#endif
> +
>  static drmServerInfoPtr drm_server_info;
>
>  void drmSetServerInfo(drmServerInfoPtr info)
> diff --git a/xf86drm.h b/xf86drm.h
> index 481d882..1d45f02 100644
> --- a/xf86drm.h
> +++ b/xf86drm.h
> @@ -58,7 +58,11 @@ extern "C" {
>
>  #else /* One of the *BSDs */
>
> +#if defined(__GNU__)
> +#include <mach/i386/ioccom.h>
The porting guidelines [1] does mention that ioctl.h is available
(although in the context of _IOT/_IOTS), so I'm wondering how come the
IOC_* symbols are not in there. Is there any documentation that says
that which header should be used ?

With the "i386" in the path, the above does look rather hacky. And
looking at the current code - can you please move the "one of the
BSDs" include at the top (before the extern C guard). The GNU one
should also be there.

Thanks
Emil
Hi Tobias,

On 24 April 2016 at 09:09, Tobias Frost <tobi@coldtobi.de> wrote:
> As both BSD and Hurd needs also the typedefs, so making sure that they are
> become declared when not compiling for linux.
> ---
>  include/drm/drm.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/include/drm/drm.h b/include/drm/drm.h
> index b4ebaa9..e77146f 100644
> --- a/include/drm/drm.h
> +++ b/include/drm/drm.h
> @@ -42,10 +42,14 @@
>  #include <asm/ioctl.h>
>  typedef unsigned int drm_handle_t;
>
> +#elif defined(__GNU__) /* this is hurd */
> +#include <stdint.h>
> +#include <mach/i386/ioccom.h>
>  #else /* One of the BSDs */
> -
>  #include <sys/ioccom.h>
>  #include <sys/types.h>
> +#endif
> +#if !defined(__linux__)

The patch should be based against the linux kernel. Namely

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/drm/drm.h

I would kindly ask that you familiarise yourself with the
SubmittingPatches document [1] if you haven't already
But before anything, can we hear please on the "using i386 in the
include feels hacky" topic mentioned in the other patch.

Thanks
Emil

[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches