[Xenomai-core] Analogy regressions in xenomai 2.5.5.1

[Xenomai-core] Analogy regressions in xenomai 2.5.5.1

[Daniele Nicolodi]

Hello, I just installed xenomai 2.5.5 on a 2.6.35.7 kernel and I'm
noticing serious regressions on analogy basic features:

1. Buffer management is badly broken. It is not possible to run any
acquisition command that wraps around in the ring buffer. That can be
simply reproduced with:

cmd_read -v d analogy0 -s 0 -S 0

after a while it will fail with: "cmd_read: a4l_read failed (ret=-32)".
In dmesg the driver reports: "Analogy: MITE: DME overwrite of free area".

2. Buffer is kept memory mapped after the mapping process dies. If a
process mapping a device buffer dies before un-mapping the buffer, it is
not possible to reconfigure the bnuffer. a4l_set_bufsize fails with
error code 32 and dmesg read "Analogy: a4l_ioctl_bufcfg: please unmap
before configuring buffer". Sinche the mapping process died I do not
know how to do this.

When I proposed some easy changes to the analogy API, to make it much
less confusing with trivial changes, Alexis expressed concerns about API
stability. What about functional stability? I think the reported ones
are major problems easily catch with simple pre-release tests...

Cheers,
Daniele

Re: [Xenomai-core] Analogy regressions in xenomai 2.5.5.1

[Philippe Gerum]

On Tue, 2010-10-12 at 10:44 +0200, Daniele Nicolodi wrote:
> Hello, I just installed xenomai 2.5.5 on a 2.6.35.7 kernel and I'm
> noticing serious regressions on analogy basic features:
> 
> 1. Buffer management is badly broken. It is not possible to run any
> acquisition command that wraps around in the ring buffer. That can be
> simply reproduced with:
> 
> cmd_read -v d analogy0 -s 0 -S 0
> 
> after a while it will fail with: "cmd_read: a4l_read failed (ret=-32)".
> In dmesg the driver reports: "Analogy: MITE: DME overwrite of free area".
> 
> 2. Buffer is kept memory mapped after the mapping process dies. If a
> process mapping a device buffer dies before un-mapping the buffer, it is
> not possible to reconfigure the bnuffer. a4l_set_bufsize fails with
> error code 32 and dmesg read "Analogy: a4l_ioctl_bufcfg: please unmap
> before configuring buffer". Sinche the mapping process died I do not
> know how to do this.
> 
> When I proposed some easy changes to the analogy API, to make it much
> less confusing with trivial changes, Alexis expressed concerns about API
> stability. What about functional stability? I think the reported ones
> are major problems easily catch with simple pre-release tests...

Do not hesitate to provide them.

> 
> Cheers,
> Daniele
> 
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core

-- 
Philippe.

Re: [Xenomai-core] Analogy regressions in xenomai 2.5.5.1

[Daniele Nicolodi]

On 12/10/10 10:56, Philippe Gerum wrote:
>> When I proposed some easy changes to the analogy API, to make it much
>> less confusing with trivial changes, Alexis expressed concerns about API
>> stability. What about functional stability? I think the reported ones
>> are major problems easily catch with simple pre-release tests...
> 
> Do not hesitate to provide them.

I'll try to write something later today. There is a testing
infrastructure where those test should fit? I haven't found one in
xenomai source code.

Cheers,
-- 
Daniele

Re: [Xenomai-core] Analogy regressions in xenomai 2.5.5.1

[Gilles Chanteperdrix]

Daniele Nicolodi wrote:
> On 12/10/10 10:56, Philippe Gerum wrote:
>>> When I proposed some easy changes to the analogy API, to make it much
>>> less confusing with trivial changes, Alexis expressed concerns about API
>>> stability. What about functional stability? I think the reported ones
>>> are major problems easily catch with simple pre-release tests...
>> Do not hesitate to provide them.
> 
> I'll try to write something later today. There is a testing
> infrastructure where those test should fit? I haven't found one in
> xenomai source code.

Please send the test cases, we will take care of the rest.

-- 
					    Gilles.

Re: [Xenomai-core] Analogy regressions in xenomai 2.5.5.1

[Alexis Berlemont]

Hi,

On Tue, Oct 12, 2010 at 10:44 AM, Daniele Nicolodi <daniele@domain.hid> wrote:
> Hello, I just installed xenomai 2.5.5 on a 2.6.35.7 kernel and I'm
> noticing serious regressions on analogy basic features:
>
> 1. Buffer management is badly broken. It is not possible to run any
> acquisition command that wraps around in the ring buffer. That can be
> simply reproduced with:
>
> cmd_read -v d analogy0 -s 0 -S 0
>
> after a while it will fail with: "cmd_read: a4l_read failed (ret=-32)".
> In dmesg the driver reports: "Analogy: MITE: DME overwrite of free area".

OK. Maybe, it is not Analogy's buffer management; it may be specific
to the NI driver. Did you try to reproduce the bug with another driver
(analogy_fake for example)? It may be due to some fix I made this
summer (which is surprising, I thought I tested the change quite
exhaustively... Sorry for that).

>
> 2. Buffer is kept memory mapped after the mapping process dies. If a
> process mapping a device buffer dies before un-mapping the buffer, it is
> not possible to reconfigure the bnuffer. a4l_set_bufsize fails with
> error code 32 and dmesg read "Analogy: a4l_ioctl_bufcfg: please unmap
> before configuring buffer". Sinche the mapping process died I do not
> know how to do this.
>
With a 2.4.x release, did you manage to reproduce the bug?

> When I proposed some easy changes to the analogy API, to make it much
> less confusing with trivial changes, Alexis expressed concerns about API
> stability. What about functional stability? I think the reported ones
> are major problems easily catch with simple pre-release tests...
>
> Cheers,
> Daniele
>
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core
>

Alexis.

Re: [Xenomai-core] Analogy regressions in xenomai 2.5.5.1

[Daniele Nicolodi]

[-- Attachment #1: Type: text/plain, Size: 1579 bytes --]

On 13/10/10 08:31, Alexis Berlemont wrote:
>> 1. Buffer management is badly broken. It is not possible to run any
>> acquisition command that wraps around in the ring buffer. That can be
>> simply reproduced with:
>>
>> cmd_read -v d analogy0 -s 0 -S 0
>>
>> after a while it will fail with: "cmd_read: a4l_read failed (ret=-32)".
>> In dmesg the driver reports: "Analogy: MITE: DME overwrite of free area".
> 
> OK. Maybe, it is not Analogy's buffer management; it may be specific
> to the NI driver. Did you try to reproduce the bug with another driver
> (analogy_fake for example)? It may be due to some fix I made this
> summer (which is surprising, I thought I tested the change quite
> exhaustively... Sorry for that).

The problem is probably in the NI driver. I'm unable to reproduce the
bug with the analogy_fake driver. See attached test script.

>> 2. Buffer is kept memory mapped after the mapping process dies. If a
>> process mapping a device buffer dies before un-mapping the buffer, it is
>> not possible to reconfigure the bnuffer. a4l_set_bufsize fails with
>> error code 32 and dmesg read "Analogy: a4l_ioctl_bufcfg: please unmap
>> before configuring buffer". Sinche the mapping process died I do not
>> know how to do this.
>>
> With a 2.4.x release, did you manage to reproduce the bug?

I'm unable to reproduce the bug with xenomai 2.5.3. See attached test.
It depends on a patch to make the --read-buffer-size and
--write-buffer-size options to analogy_config to effectively work. Also
attached. I do not have a 2.5.4 version handy.

Cheers,
-- 
Daniele


[-- Attachment #2: test_buffer_bug.sh --]
[-- Type: application/x-sh, Size: 509 bytes --]

[-- Attachment #3: test_mmap_bug.sh --]
[-- Type: application/x-sh, Size: 442 bytes --]

[-- Attachment #4: analogy_mmap_test.c --]
[-- Type: text/plain, Size: 1366 bytes --]

/**
 * analogy for linux - input command test program
 */

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/mman.h>
#include <errno.h>
#include <getopt.h>
#include <string.h>

#include <analogy/analogy.h>

/* default device name */
#define DEVICE "analogy0"

#define DEBUG(frmt, args...) fprintf(stderr, frmt"\n", ##args)
#define ERROR(frmt, args...) fprintf(stderr, frmt"\n", ##args)

int main(int argc, char **argv)
{
     int rv = 0;
     unsigned long bufsize;
     void *map = NULL;
     a4l_desc_t dsc;

     char *filename = argv[1];

     /* open the device */
     rv = a4l_open(&dsc, filename);
     if (rv < 0) {	  
	  ERROR("analogy open");
	  return rv;
     }
                    
     /* cancel any command which might be in progress */
     a4l_snd_cancel(&dsc, dsc.idx_read_subd);
     
     /* get buffer size to map */
     rv = a4l_get_bufsize(&dsc, dsc.idx_read_subd, &bufsize);
     if (rv < 0) {
	  ERROR("analogy get bufsize");
	  return rv;
     }

     /* map read subdevice buffer */
     rv = a4l_mmap(&dsc, dsc.idx_read_subd, bufsize, &map);
     if (rv < 0) {
	  ERROR("analogy mmap");
	  return rv;
     }

     if (argc > 2) {
	  DEBUG("BUG!");
     } else {	  
	  /* unmap subdevice buffer */
	  munmap(map, bufsize);
     }
     
     /* close file descriptor */
     a4l_close(&dsc);
     
     exit(0);
}


[-- Attachment #5: analogy-config.patch --]
[-- Type: text/x-diff, Size: 8620 bytes --]

commit e7ac38f3f45e439a8d383f3b4adafbb97bf543fe
Author: Daniele Nicolodi <nicolodi@domain.hid>
Date:   Wed Oct 13 16:03:30 2010 +0200

    Make the --read-buffer-size and --write-buffer-size otpions effective.

diff --git a/src/utils/analogy/analogy_config.c b/src/utils/analogy/analogy_config.c
index 1482322..2ce3ddd 100644
--- a/src/utils/analogy/analogy_config.c
+++ b/src/utils/analogy/analogy_config.c
@@ -31,28 +31,25 @@
 
 #include <analogy/analogy.h>
 
-/* Declare precompilation constants */
-#define __NBMIN_ARG 2
-#define __NBMAX_ARG 3
-#define __OPTS_DELIMITER ","
+enum action {
+	ATTACH_DRIVER,
+	DETACH_DRIVER,
+	CONFIGURE_BUFFER_SIZE,
+};
 
-/* Declare prog variables */
-int vlevel = 1;
-int unatt_act = 0;
-struct option a4l_conf_opts[] = {
-	{"help", no_argument, NULL, 'h'},
-	{"verbose", no_argument, NULL, 'v'},
-	{"quiet", no_argument, NULL, 'q'},
-	{"version", no_argument, NULL, 'V'},
-	{"remove", no_argument, NULL, 'r'},
-	{"read-buffer-size", required_argument, NULL, 'R'},
-	{"write-buffer-size", required_argument, NULL, 'W'},
-	{0},
+enum subdevice {
+	READ_SUBDEVICE,
+	WRITE_SUBDEVICE,
 };
 
+/* Verbosity level */
+int vlevel = 1;
+
+/* Driver specific Options delimiter */
+#define DELIMITER ","
+
 int compute_opts(char *opts, unsigned int *nb, unsigned long *res)
 {
-
 	int ret = 0, len, ofs;
 
 	/* Check arg and inits it */
@@ -67,7 +64,7 @@ int compute_opts(char *opts, unsigned int *nb, unsigned long *res)
 	do {
 		(*nb)++;
 		len = strlen(opts);
-		ofs = strcspn(opts, __OPTS_DELIMITER);
+		ofs = strcspn(opts, DELIMITER);
 		if (res != NULL) {
 			res[(*nb) - 1] = strtoul(opts, NULL, 0);
 			if (errno != 0) {
@@ -104,79 +101,57 @@ void do_print_usage(void)
 		"\t\t -W, --write-buffer-size: write buffer size in kB\n");
 }
 
-int main(int argc, char *argv[])
+int detach_driver(const char *devfile)
 {
-	int c;
-	char *devfile;
-	a4l_lnkdesc_t lnkdsc;
-	int chk_nb, ret = 0, fd = -1;
-
-	/* Init the descriptor structure */
-	memset(&lnkdsc, 0, sizeof(a4l_lnkdesc_t));
-
-	/* Compute arguments */
-	while ((c =
-		getopt_long(argc, argv, "hvqVrR:W:", a4l_conf_opts,
-			    NULL)) >= 0) {
-		switch (c) {
-		case 'h':
-			do_print_usage();
-			goto out_a4l_config;
-		case 'v':
-			vlevel = 2;
-			break;
-		case 'q':
-			vlevel = 0;
-			break;
-		case 'V':
-			do_print_version();
-			goto out_a4l_config;
-		case 'r':
-			unatt_act = 1;
-			break;
-		case 'R':
-			break;
-		case 'W':
-			break;
-		}
+	/* Open device */
+	int fd = a4l_sys_open(devfile);
+	if (fd < 0) {
+		fprintf(stderr, "a4l_open failed ret=%d\n", fd);
+		return fd;
 	}
 
-	/* Check the last arguments */
-	chk_nb = (unatt_act == 0) ? __NBMIN_ARG : __NBMIN_ARG - 1;
-	if (argc - optind < chk_nb) {
-		do_print_usage();
-		goto out_a4l_config;
+	/* Detach driver */
+	int ret = a4l_sys_detach(fd);
+	if (ret < 0) {
+		fprintf(stderr, "a4l_snd_detach failed ret=%d\n", ret);
+		return ret;
 	}
+	
+	/* Close device */
+	a4l_sys_close(fd);
 
-	/* Get the device file name */
-	devfile = argv[optind];
+	return 0;
+}
+
+int attach_driver(const char *devfile, char *driver, char *opts)
+{	
+	int ret;
+	a4l_lnkdesc_t lnkdsc;
+
+	/* Init the descriptor structure */
+	memset(&lnkdsc, 0, sizeof(a4l_lnkdesc_t));
 
 	/* Fill the descriptor with the driver name */
-	if (unatt_act == 0) {
-		lnkdsc.bname = argv[optind + 1];
-		lnkdsc.bname_size = strlen(argv[optind + 1]);
-	}
+	lnkdsc.bname = driver;
+	lnkdsc.bname_size = strlen(driver);
 
-	/* Handle the last arguments: the driver-specific args */
-	if (unatt_act == 1 || argc - optind != __NBMAX_ARG)
-		lnkdsc.opts_size = 0;
-	else {
-		char *opts = argv[optind + __NBMAX_ARG - 1];
+	/* Handle driver specific options */
+	lnkdsc.opts_size = 0;
+	if (opts != NULL) {
 		if ((ret = compute_opts(opts, &lnkdsc.opts_size, NULL)) < 0) {
 			fprintf(stderr,
 				"analogy_config: specific-driver options recovery failed\n");
 			fprintf(stderr,
 				"\twarning: specific-driver options must be integer value\n");
 			do_print_usage();
-			goto out_a4l_config;
+			return -1;
 		}
 
 		lnkdsc.opts = malloc(lnkdsc.opts_size);
 		if (lnkdsc.opts == NULL) {
 			fprintf(stderr,
 				"analogy_config: memory allocation failed\n");
-			ret = -ENOMEM;
-			goto out_a4l_config;
+			return -ENOMEM;
 		}
 
 		if ((ret =
@@ -186,38 +161,165 @@ int main(int argc, char *argv[])
 			fprintf(stderr,
 				"\twarning: specific-driver options must be integer value\n");
 			do_print_usage();
-			goto out_a4l_config;
+			return -1;
 		}
 	}
 
 	/* Open the specified file */
-	fd = a4l_sys_open(devfile);
+	int fd = a4l_sys_open(devfile);
 	if (fd < 0) {
-		ret = fd;
-		fprintf(stderr, "analogy_config: a4l_open failed ret=%d\n",
-			ret);
-		goto out_a4l_config;
+		fprintf(stderr, "a4l_open failed ret=%d\n", fd);
+		return fd;
 	}
 
-	/* Trigger the ioctl */
-	if (unatt_act == 0)
-		ret = a4l_sys_attach(fd, &lnkdsc);
-	else
-		ret = a4l_sys_detach(fd);
+	/* Attach driver */
+	ret = a4l_sys_attach(fd, &lnkdsc);
 	if (ret < 0) {
-		fprintf(stderr, "analogy_config: %s failed ret=%d\n",
-			(unatt_act ==
-			 0) ? "a4l_snd_attach" : "a4l_snd_detach", ret);
-		goto out_a4l_config;
+		fprintf(stderr, "a4l_snd_attach failed ret=%d\n", ret);
+		return ret;
 	}
 
-out_a4l_config:
+	/* Close device */	
+	a4l_sys_close(fd);
 
-	if (fd >= 0)
-		a4l_sys_close(fd);
+	free(lnkdsc.opts);
 
-	if (lnkdsc.opts != NULL)
-		free(lnkdsc.opts);
+	return 0;
+}
 
-	return ret;
+int configure_buffer_size(char *device, int subd, unsigned long size)
+{
+	/* Open device */
+	a4l_desc_t dsc;
+	int ret = a4l_open(&dsc, device);
+	if (ret < 0) {
+		fprintf(stderr, "a4l_open error ret=%d\n", ret);
+		return ret;
+	}
+
+	/* Choose subdevice */
+	unsigned long subd_idx = (subd == READ_SUBDEVICE ? dsc.idx_read_subd : dsc.idx_write_subd);
+	const char *name = (subd == READ_SUBDEVICE ? "read" : "write");
+
+	/* Get buffer size */
+	unsigned long bufsize;
+	ret = a4l_get_bufsize(&dsc, subd_idx, &bufsize);
+	if (ret < 0) {
+		fprintf(stderr, "a4l_get_bufsize error ret=%d\n", ret);
+		return ret;
+	}
+
+	if (size > 0) {
+
+		/* Configure buffer size */
+		ret = a4l_set_bufsize(&dsc, subd_idx, size);
+		if (ret < 0) {
+			fprintf(stderr, "a4l_set_bufsize error ret=%d\n", ret);
+			return -1;
+		}
+
+		/* Report buffer size */
+		if (vlevel > 1) {
+			fprintf(stdout, "old %s buffer size = %lu\n", name, bufsize);
+			fprintf(stdout, "new %s buffer size = %lu\n", name, size);
+		}
+
+	} else {
+
+		/* Report buffer size */
+		if (vlevel > 0)
+			fprintf(stdout, "%s buffer size = %lu\n", name, bufsize);
+		else
+			fprintf(stdout, "%lu\n", bufsize);
+
+	}
+
+	/* Close device */
+	ret = a4l_close(&dsc);
+	if (ret < 0) {
+		fprintf(stderr, "a4l_close error ret=%d\n", ret);
+		return -1;
+	}
+
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	int c;
+	int action;
+	int subdevice = -1;
+	unsigned long size = 0;
+
+	/* Default action is to attach a driver */
+	action = ATTACH_DRIVER;
+
+	struct option options[] = {
+		{"help",              no_argument,       NULL, 'h'},
+		{"verbose",           no_argument,       NULL, 'v'},
+		{"quiet",             no_argument,       NULL, 'q'},
+		{"version",           no_argument,       NULL, 'V'},
+		{"remove",            no_argument,       NULL, 'r'},
+		{"read-buffer-size",  optional_argument, NULL, 'R'},
+		{"write-buffer-size", optional_argument, NULL, 'W'},
+		{ 0 },
+	};
+
+	/* Compute arguments */
+	while ((c =
+		getopt_long(argc, argv, "hvqVrR::W::", options, NULL)) >= 0) {
+		switch (c) {
+		case 'h':
+			do_print_usage();
+			return 0;
+		case 'v':
+			vlevel = 2;
+			break;
+		case 'q':
+			vlevel = 0;
+			break;
+		case 'V':
+			do_print_version();
+			return 0;
+		case 'r':
+			action = DETACH_DRIVER;
+			break;
+		case 'R':
+			action = CONFIGURE_BUFFER_SIZE;
+			subdevice = READ_SUBDEVICE;
+			if (optarg)
+				size = strtoul(optarg, NULL, 10);
+			break;
+		case 'W':
+			action = CONFIGURE_BUFFER_SIZE;
+			subdevice = WRITE_SUBDEVICE;
+			if (optarg)
+				size = strtoul(optarg, NULL, 10);
+			break;
+		}
+	}
+
+	switch (action) {
+	case ATTACH_DRIVER:
+		if ((argc - optind) == 2)
+			return attach_driver(argv[optind], argv[optind+1], NULL);
+		if ((argc - optind) == 3)
+			return attach_driver(argv[optind], argv[optind+1], argv[optind+2]);
+		do_print_usage();
+		return -1;
+		
+	case DETACH_DRIVER:
+		if ((argc - optind) == 1)
+			return detach_driver(argv[optind]);
+		do_print_usage();
+		return -1;
+
+	case CONFIGURE_BUFFER_SIZE:
+		if ((argc - optind) == 1)
+			return configure_buffer_size(argv[optind], subdevice, size);
+		do_print_usage();
+		return -1;
+	}
+
+	return 0;
 }

Re: [Xenomai-core] Analogy regressions in xenomai 2.5.5.1

[Daniele Nicolodi]

Hi Alexis,

do you have the possibility to look at those bugs soonish? Otherwise I
can try to fix them, but I would need some hints on where I should look
in the code.

Thank you. Cheers,
-- 
Daniele

Re: [Xenomai-core] Analogy regressions in xenomai 2.5.5.1

[Alexis Berlemont]

Hi,

Do not worry. I have not forgotten.

On Fri, Oct 15, 2010 at 11:16 AM, Daniele Nicolodi <daniele@domain.hid> wrote:
> Hi Alexis,
>
> do you have the possibility to look at those bugs soonish? Otherwise I
> can try to fix them, but I would need some hints on where I should look
> in the code.
>
> Thank you. Cheers,
> --
> Daniele
>
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core
>

Alexis.

Re: [Xenomai-core] Analogy regressions in xenomai 2.5.5.1

[Daniele Nicolodi]

On 15/10/10 12:15, Alexis Berlemont wrote:
> Hi,
> 
> Do not worry. I have not forgotten.

Sorry, I didn't want to imply that you forgot about the issues.

It was just to let you know that I'm willing to work on the issues, but
I do not know the code well enough to be able to isolate the issue
without spending quite a lot of time. If you give me an hint I'll be
happy to help, in a way compatible to my PhD thesis writing... :-)

I think analogy is a nice project, and I think that with xenomai 3.0
where the RTDM layer would not depend anymore on ipipe I think it has
the potential to overtake comedi, and I would like to invest more time
on it.

Cheers,
-- 
Daniele

Re: [Xenomai-core] Analogy regressions in xenomai 2.5.5.1

[Alexis Berlemont]

Hi,

Daniele Nicolodi wrote:
> On 13/10/10 08:31, Alexis Berlemont wrote:
> >> 1. Buffer management is badly broken. It is not possible to run any
> >> acquisition command that wraps around in the ring buffer. That can be
> >> simply reproduced with:
> >>
> >> cmd_read -v d analogy0 -s 0 -S 0
> >>
> >> after a while it will fail with: "cmd_read: a4l_read failed (ret=-32)".
> >> In dmesg the driver reports: "Analogy: MITE: DME overwrite of free area".
> > 
> > OK. Maybe, it is not Analogy's buffer management; it may be specific
> > to the NI driver. Did you try to reproduce the bug with another driver
> > (analogy_fake for example)? It may be due to some fix I made this
> > summer (which is surprising, I thought I tested the change quite
> > exhaustively... Sorry for that).
> 
> The problem is probably in the NI driver. I'm unable to reproduce the
> bug with the analogy_fake driver. See attached test script.
> 

The bug was not located in the NI driver but in the wrong setting of
the default size of the buffer. It is fixed now. If you have time to
cross-check on your board, that would be nice (xenomai-abe / branch
analogy). 

> >> 2. Buffer is kept memory mapped after the mapping process dies. If a
> >> process mapping a device buffer dies before un-mapping the buffer, it is
> >> not possible to reconfigure the bnuffer. a4l_set_bufsize fails with
> >> error code 32 and dmesg read "Analogy: a4l_ioctl_bufcfg: please unmap
> >> before configuring buffer". Sinche the mapping process died I do not
> >> know how to do this.
> >>
> > With a 2.4.x release, did you manage to reproduce the bug?
> 
> I'm unable to reproduce the bug with xenomai 2.5.3. See attached test.
> It depends on a patch to make the --read-buffer-size and
> --write-buffer-size options to analogy_config to effectively work. Also
> attached. I do not have a 2.5.4 version handy.
> 
> Cheers,
> -- 
> Daniele
> 



> /**
>  * analogy for linux - input command test program
>  */
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/mman.h>
> #include <errno.h>
> #include <getopt.h>
> #include <string.h>
> 
> #include <analogy/analogy.h>
> 
> /* default device name */
> #define DEVICE "analogy0"
> 
> #define DEBUG(frmt, args...) fprintf(stderr, frmt"\n", ##args)
> #define ERROR(frmt, args...) fprintf(stderr, frmt"\n", ##args)
> 
> int main(int argc, char **argv)
> {
>      int rv = 0;
>      unsigned long bufsize;
>      void *map = NULL;
>      a4l_desc_t dsc;
> 
>      char *filename = argv[1];
> 
>      /* open the device */
>      rv = a4l_open(&dsc, filename);
>      if (rv < 0) {	  
> 	  ERROR("analogy open");
> 	  return rv;
>      }
>                     
>      /* cancel any command which might be in progress */
>      a4l_snd_cancel(&dsc, dsc.idx_read_subd);
>      
>      /* get buffer size to map */
>      rv = a4l_get_bufsize(&dsc, dsc.idx_read_subd, &bufsize);
>      if (rv < 0) {
> 	  ERROR("analogy get bufsize");
> 	  return rv;
>      }
> 
>      /* map read subdevice buffer */
>      rv = a4l_mmap(&dsc, dsc.idx_read_subd, bufsize, &map);
>      if (rv < 0) {
> 	  ERROR("analogy mmap");
> 	  return rv;
>      }
> 
>      if (argc > 2) {
> 	  DEBUG("BUG!");
>      } else {	  
> 	  /* unmap subdevice buffer */
> 	  munmap(map, bufsize);
>      }
>      
>      /* close file descriptor */
>      a4l_close(&dsc);
>      
>      exit(0);
> }
> 

> commit e7ac38f3f45e439a8d383f3b4adafbb97bf543fe
> Author: Daniele Nicolodi <nicolodi@domain.hid>
> Date:   Wed Oct 13 16:03:30 2010 +0200
> 
>     Make the --read-buffer-size and --write-buffer-size otpions effective.
> 
> diff --git a/src/utils/analogy/analogy_config.c b/src/utils/analogy/analogy_config.c
> index 1482322..2ce3ddd 100644
> --- a/src/utils/analogy/analogy_config.c
> +++ b/src/utils/analogy/analogy_config.c
> @@ -31,28 +31,25 @@
>  
>  #include <analogy/analogy.h>
>  
> -/* Declare precompilation constants */
> -#define __NBMIN_ARG 2
> -#define __NBMAX_ARG 3
> -#define __OPTS_DELIMITER ","
> +enum action {
> +	ATTACH_DRIVER,
> +	DETACH_DRIVER,
> +	CONFIGURE_BUFFER_SIZE,
> +};
>  
> -/* Declare prog variables */
> -int vlevel = 1;
> -int unatt_act = 0;
> -struct option a4l_conf_opts[] = {
> -	{"help", no_argument, NULL, 'h'},
> -	{"verbose", no_argument, NULL, 'v'},
> -	{"quiet", no_argument, NULL, 'q'},
> -	{"version", no_argument, NULL, 'V'},
> -	{"remove", no_argument, NULL, 'r'},
> -	{"read-buffer-size", required_argument, NULL, 'R'},
> -	{"write-buffer-size", required_argument, NULL, 'W'},
> -	{0},
> +enum subdevice {
> +	READ_SUBDEVICE,
> +	WRITE_SUBDEVICE,
>  };
>  
> +/* Verbosity level */
> +int vlevel = 1;
> +
> +/* Driver specific Options delimiter */
> +#define DELIMITER ","
> +
>  int compute_opts(char *opts, unsigned int *nb, unsigned long *res)
>  {
> -
>  	int ret = 0, len, ofs;
>  
>  	/* Check arg and inits it */
> @@ -67,7 +64,7 @@ int compute_opts(char *opts, unsigned int *nb, unsigned long *res)
>  	do {
>  		(*nb)++;
>  		len = strlen(opts);
> -		ofs = strcspn(opts, __OPTS_DELIMITER);
> +		ofs = strcspn(opts, DELIMITER);
>  		if (res != NULL) {
>  			res[(*nb) - 1] = strtoul(opts, NULL, 0);
>  			if (errno != 0) {
> @@ -104,79 +101,57 @@ void do_print_usage(void)
>  		"\t\t -W, --write-buffer-size: write buffer size in kB\n");
>  }
>  
> -int main(int argc, char *argv[])
> +int detach_driver(const char *devfile)
>  {
> -	int c;
> -	char *devfile;
> -	a4l_lnkdesc_t lnkdsc;
> -	int chk_nb, ret = 0, fd = -1;
> -
> -	/* Init the descriptor structure */
> -	memset(&lnkdsc, 0, sizeof(a4l_lnkdesc_t));
> -
> -	/* Compute arguments */
> -	while ((c =
> -		getopt_long(argc, argv, "hvqVrR:W:", a4l_conf_opts,
> -			    NULL)) >= 0) {
> -		switch (c) {
> -		case 'h':
> -			do_print_usage();
> -			goto out_a4l_config;
> -		case 'v':
> -			vlevel = 2;
> -			break;
> -		case 'q':
> -			vlevel = 0;
> -			break;
> -		case 'V':
> -			do_print_version();
> -			goto out_a4l_config;
> -		case 'r':
> -			unatt_act = 1;
> -			break;
> -		case 'R':
> -			break;
> -		case 'W':
> -			break;
> -		}
> +	/* Open device */
> +	int fd = a4l_sys_open(devfile);
> +	if (fd < 0) {
> +		fprintf(stderr, "a4l_open failed ret=%d\n", fd);
> +		return fd;
>  	}
>  
> -	/* Check the last arguments */
> -	chk_nb = (unatt_act == 0) ? __NBMIN_ARG : __NBMIN_ARG - 1;
> -	if (argc - optind < chk_nb) {
> -		do_print_usage();
> -		goto out_a4l_config;
> +	/* Detach driver */
> +	int ret = a4l_sys_detach(fd);
> +	if (ret < 0) {
> +		fprintf(stderr, "a4l_snd_detach failed ret=%d\n", ret);
> +		return ret;
>  	}
> +	
> +	/* Close device */
> +	a4l_sys_close(fd);
>  
> -	/* Get the device file name */
> -	devfile = argv[optind];
> +	return 0;
> +}
> +
> +int attach_driver(const char *devfile, char *driver, char *opts)
> +{	
> +	int ret;
> +	a4l_lnkdesc_t lnkdsc;
> +
> +	/* Init the descriptor structure */
> +	memset(&lnkdsc, 0, sizeof(a4l_lnkdesc_t));
>  
>  	/* Fill the descriptor with the driver name */
> -	if (unatt_act == 0) {
> -		lnkdsc.bname = argv[optind + 1];
> -		lnkdsc.bname_size = strlen(argv[optind + 1]);
> -	}
> +	lnkdsc.bname = driver;
> +	lnkdsc.bname_size = strlen(driver);
>  
> -	/* Handle the last arguments: the driver-specific args */
> -	if (unatt_act == 1 || argc - optind != __NBMAX_ARG)
> -		lnkdsc.opts_size = 0;
> -	else {
> -		char *opts = argv[optind + __NBMAX_ARG - 1];
> +	/* Handle driver specific options */
> +	lnkdsc.opts_size = 0;
> +	if (opts != NULL) {
>  		if ((ret = compute_opts(opts, &lnkdsc.opts_size, NULL)) < 0) {
>  			fprintf(stderr,
>  				"analogy_config: specific-driver options recovery failed\n");
>  			fprintf(stderr,
>  				"\twarning: specific-driver options must be integer value\n");
>  			do_print_usage();
> -			goto out_a4l_config;
> +			return -1;
>  		}
>  
>  		lnkdsc.opts = malloc(lnkdsc.opts_size);
>  		if (lnkdsc.opts == NULL) {
>  			fprintf(stderr,
>  				"analogy_config: memory allocation failed\n");
> -			ret = -ENOMEM;
> -			goto out_a4l_config;
> +			return -ENOMEM;
>  		}
>  
>  		if ((ret =
> @@ -186,38 +161,165 @@ int main(int argc, char *argv[])
>  			fprintf(stderr,
>  				"\twarning: specific-driver options must be integer value\n");
>  			do_print_usage();
> -			goto out_a4l_config;
> +			return -1;
>  		}
>  	}
>  
>  	/* Open the specified file */
> -	fd = a4l_sys_open(devfile);
> +	int fd = a4l_sys_open(devfile);
>  	if (fd < 0) {
> -		ret = fd;
> -		fprintf(stderr, "analogy_config: a4l_open failed ret=%d\n",
> -			ret);
> -		goto out_a4l_config;
> +		fprintf(stderr, "a4l_open failed ret=%d\n", fd);
> +		return fd;
>  	}
>  
> -	/* Trigger the ioctl */
> -	if (unatt_act == 0)
> -		ret = a4l_sys_attach(fd, &lnkdsc);
> -	else
> -		ret = a4l_sys_detach(fd);
> +	/* Attach driver */
> +	ret = a4l_sys_attach(fd, &lnkdsc);
>  	if (ret < 0) {
> -		fprintf(stderr, "analogy_config: %s failed ret=%d\n",
> -			(unatt_act ==
> -			 0) ? "a4l_snd_attach" : "a4l_snd_detach", ret);
> -		goto out_a4l_config;
> +		fprintf(stderr, "a4l_snd_attach failed ret=%d\n", ret);
> +		return ret;
>  	}
>  
> -out_a4l_config:
> +	/* Close device */	
> +	a4l_sys_close(fd);
>  
> -	if (fd >= 0)
> -		a4l_sys_close(fd);
> +	free(lnkdsc.opts);
>  
> -	if (lnkdsc.opts != NULL)
> -		free(lnkdsc.opts);
> +	return 0;
> +}
>  
> -	return ret;
> +int configure_buffer_size(char *device, int subd, unsigned long size)
> +{
> +	/* Open device */
> +	a4l_desc_t dsc;
> +	int ret = a4l_open(&dsc, device);
> +	if (ret < 0) {
> +		fprintf(stderr, "a4l_open error ret=%d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Choose subdevice */
> +	unsigned long subd_idx = (subd == READ_SUBDEVICE ? dsc.idx_read_subd : dsc.idx_write_subd);
> +	const char *name = (subd == READ_SUBDEVICE ? "read" : "write");
> +
> +	/* Get buffer size */
> +	unsigned long bufsize;
> +	ret = a4l_get_bufsize(&dsc, subd_idx, &bufsize);
> +	if (ret < 0) {
> +		fprintf(stderr, "a4l_get_bufsize error ret=%d\n", ret);
> +		return ret;
> +	}
> +
> +	if (size > 0) {
> +
> +		/* Configure buffer size */
> +		ret = a4l_set_bufsize(&dsc, subd_idx, size);
> +		if (ret < 0) {
> +			fprintf(stderr, "a4l_set_bufsize error ret=%d\n", ret);
> +			return -1;
> +		}
> +
> +		/* Report buffer size */
> +		if (vlevel > 1) {
> +			fprintf(stdout, "old %s buffer size = %lu\n", name, bufsize);
> +			fprintf(stdout, "new %s buffer size = %lu\n", name, size);
> +		}
> +
> +	} else {
> +
> +		/* Report buffer size */
> +		if (vlevel > 0)
> +			fprintf(stdout, "%s buffer size = %lu\n", name, bufsize);
> +		else
> +			fprintf(stdout, "%lu\n", bufsize);
> +
> +	}
> +
> +	/* Close device */
> +	ret = a4l_close(&dsc);
> +	if (ret < 0) {
> +		fprintf(stderr, "a4l_close error ret=%d\n", ret);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	int c;
> +	int action;
> +	int subdevice = -1;
> +	unsigned long size = 0;
> +
> +	/* Default action is to attach a driver */
> +	action = ATTACH_DRIVER;
> +
> +	struct option options[] = {
> +		{"help",              no_argument,       NULL, 'h'},
> +		{"verbose",           no_argument,       NULL, 'v'},
> +		{"quiet",             no_argument,       NULL, 'q'},
> +		{"version",           no_argument,       NULL, 'V'},
> +		{"remove",            no_argument,       NULL, 'r'},
> +		{"read-buffer-size",  optional_argument, NULL, 'R'},
> +		{"write-buffer-size", optional_argument, NULL, 'W'},
> +		{ 0 },
> +	};
> +
> +	/* Compute arguments */
> +	while ((c =
> +		getopt_long(argc, argv, "hvqVrR::W::", options, NULL)) >= 0) {
> +		switch (c) {
> +		case 'h':
> +			do_print_usage();
> +			return 0;
> +		case 'v':
> +			vlevel = 2;
> +			break;
> +		case 'q':
> +			vlevel = 0;
> +			break;
> +		case 'V':
> +			do_print_version();
> +			return 0;
> +		case 'r':
> +			action = DETACH_DRIVER;
> +			break;
> +		case 'R':
> +			action = CONFIGURE_BUFFER_SIZE;
> +			subdevice = READ_SUBDEVICE;
> +			if (optarg)
> +				size = strtoul(optarg, NULL, 10);
> +			break;
> +		case 'W':
> +			action = CONFIGURE_BUFFER_SIZE;
> +			subdevice = WRITE_SUBDEVICE;
> +			if (optarg)
> +				size = strtoul(optarg, NULL, 10);
> +			break;
> +		}
> +	}
> +
> +	switch (action) {
> +	case ATTACH_DRIVER:
> +		if ((argc - optind) == 2)
> +			return attach_driver(argv[optind], argv[optind+1], NULL);
> +		if ((argc - optind) == 3)
> +			return attach_driver(argv[optind], argv[optind+1], argv[optind+2]);
> +		do_print_usage();
> +		return -1;
> +		
> +	case DETACH_DRIVER:
> +		if ((argc - optind) == 1)
> +			return detach_driver(argv[optind]);
> +		do_print_usage();
> +		return -1;
> +
> +	case CONFIGURE_BUFFER_SIZE:
> +		if ((argc - optind) == 1)
> +			return configure_buffer_size(argv[optind], subdevice, size);
> +		do_print_usage();
> +		return -1;
> +	}
> +
> +	return 0;
>  }
> 

> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core


-- 
Alexis.