Re: [PATCH] xf86-input-synaptics: fix creation of repeater fifo

[Magnus Kessler]

On Sunday 28 September 2008, Peter Hutterer wrote:
> On Sun, Sep 28, 2008 at 10:09:01AM +0100, Magnus Kessler wrote:
> > The creation of the repeater fifo in the synaptics driver looks
> > dubious. The file mode should be ORed with the S_IFIFO flag and the dev
> > parameter should be null. The mknod(3p) man page suggests using mkfifo
> > instead.
>
> Agreed.
>
> >[...]
> >     repeater = xf86SetStrOption(local->options, "Repeater", NULL);
> >     if (repeater) {
> >     /* create repeater fifo */
> >-    status = mknod(repeater, 666, S_IFIFO);
> >-    if ((status != 0) && (status != EEXIST)) {
> >+    if (mkfifo(repeater, S_IWUSR|S_IRUSR|S_IWGRP|S_IRGRP|S_IWOTH|S_IROTH))
> > { xf86Msg(X_ERROR, "%s can't create repeater fifo\n", local->name); }
> > else {
> >         /* open the repeater fifo */
>
> Any special reason for dropping the EEXIST case?
>
> Cheers,
>   Peter

From mkfifo(3p):

Upon successful completion, 0 shall be returned. Otherwise, -1 shall be 
returned, no FIFO shall be created, and errno shall be set to indicate the 
error.

Testing status against EEXIST is wrong and we should check errno instead if 
we want to allow the use of an existing file. However, since we pass a file 
name that in principle could be any existing file (not just fifos) is there 
any guarantee that we can later actually use the fifo?

New patch attached.

Cheers,

Magnus

Fix creation of repeater fifo

Use mkfifo as per recommendation in mknod/mkfifo man pages with the symbolic
names for the file modes.

Mknod and mkfifo return -1 if an error occurs and set errno, which can be used
to check for existing file (EEXIST).

Signed-off-by: Magnus Kessler <[EMAIL PROTECTED]>

diff --git a/src/synaptics.c b/src/synaptics.c
index 7b5e6d5..18168e0 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -508,8 +508,8 @@ static void set_repeater_fifo(LocalDevicePtr local)
     repeater = xf86SetStrOption(local->options, "Repeater", NULL);
     if (repeater) {
 	/* create repeater fifo */
-	status = mknod(repeater, 666, S_IFIFO);
-	if ((status != 0) && (status != EEXIST)) {
+	status = mkfifo(repeater, S_IWUSR|S_IRUSR|S_IWGRP|S_IRGRP|S_IWOTH|S_IROTH);
+	if (status && (errno != EEXIST)) {
 	    xf86Msg(X_ERROR, "%s can't create repeater fifo\n", local->name);
 	} else {
 	    /* open the repeater fifo */

signature.asc
Description: This is a digitally signed message part.

_______________________________________________
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg