| Message ID | 1461216633.14744.4.camel@frost.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
--- 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;