[PATCH] IIO: Documentation: generic_buffer example: Avoid NULL pointer dereference

[PATCH] IIO: Documentation: generic_buffer example: Avoid NULL pointer dereference

[michael.hennerich]

From: Michael Hennerich <michael.hennerich@analog.com>

In case optarg n is not given return/exit

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/staging/iio/Documentation/generic_buffer.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/Documentation/generic_buffer.c b/drivers/staging/iio/Documentation/generic_buffer.c
index df23aeb..131d9d0 100644
--- a/drivers/staging/iio/Documentation/generic_buffer.c
+++ b/drivers/staging/iio/Documentation/generic_buffer.c
@@ -151,6 +151,9 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if (device_name == NULL)
+		return -1;
+
 	/* Find the device requested */
 	dev_num = find_type_by_name(device_name, "device");
 	if (dev_num < 0) {
-- 
1.6.0.2

[PATCH] IIO: Documentation: iio_utils: Avoid double free()

[michael.hennerich]

From: Michael Hennerich <michael.hennerich@analog.com>

filename is used and freed later

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/staging/iio/Documentation/iio_utils.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/iio/Documentation/iio_utils.h b/drivers/staging/iio/Documentation/iio_utils.h
index 4dc961c..3cf01a5 100644
--- a/drivers/staging/iio/Documentation/iio_utils.h
+++ b/drivers/staging/iio/Documentation/iio_utils.h
@@ -319,7 +319,6 @@ inline int build_channel_array(const char *device_dir,
 			}
 			fscanf(sysfsfp, "%u", &current->enabled);
 			fclose(sysfsfp);
-			free(filename);
 			current->scale = 1.0;
 			current->offset = 0;
 			current->name = strndup(ent->d_name,
-- 
1.6.0.2

[PATCH] IIO: Documentation: iio_utils: Fix typo

[michael.hennerich]

From: Michael Hennerich <michael.hennerich@analog.com>


Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/staging/iio/Documentation/iio_utils.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/iio/Documentation/iio_utils.h b/drivers/staging/iio/Documentation/iio_utils.h
index 3cf01a5..4b023aa 100644
--- a/drivers/staging/iio/Documentation/iio_utils.h
+++ b/drivers/staging/iio/Documentation/iio_utils.h
@@ -374,7 +374,7 @@ inline int build_channel_array(const char *device_dir,
 		}
 	}
 	/* reorder so that the array is in index order*/
-	current = malloc(sizeof(**ci_array)**counter);
+	current = malloc(sizeof(**ci_array)*(*counter));
 	if (current == NULL) {
 		ret = -ENOMEM;
 		goto error_cleanup_array;
-- 
1.6.0.2

[PATCH] IIO: Documentation: generic_buffer example: scan_size must be bytes_per_datum

[michael.hennerich]

From: Michael Hennerich <michael.hennerich@analog.com>

In case a timestamp is present, scan size returned by size_from_channelarray()
is not longer valid. However size_from_channelarray() must be still called
in order to setup locations within the buffer.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/staging/iio/Documentation/generic_buffer.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/iio/Documentation/generic_buffer.c b/drivers/staging/iio/Documentation/generic_buffer.c
index 131d9d0..c382452 100644
--- a/drivers/staging/iio/Documentation/generic_buffer.c
+++ b/drivers/staging/iio/Documentation/generic_buffer.c
@@ -226,7 +226,13 @@ int main(int argc, char **argv)
 	ret = write_sysfs_int("enable", buf_dir_name, 1);
 	if (ret < 0)
 		goto error_free_buf_dir_name;
-	scan_size = size_from_channelarray(infoarray, num_channels);
+
+	scan_size = read_sysfs_posint("bytes_per_datum", buf_dir_name);
+	if (ret < 0)
+		goto error_free_buf_dir_name;
+
+	size_from_channelarray(infoarray, num_channels);
+
 	data = malloc(scan_size*buf_len);
 	if (!data) {
 		ret = -ENOMEM;
-- 
1.6.0.2

Re: [PATCH] IIO: Documentation: iio_utils: Avoid double free()

[Jonathan Cameron]

On 02/15/11 14:46, michael.hennerich@analog.com wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> filename is used and freed later
oops.  Thanks for this. Should probably go to stable as well as into staging-next.
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
>  drivers/staging/iio/Documentation/iio_utils.h |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/staging/iio/Documentation/iio_utils.h b/drivers/staging/iio/Documentation/iio_utils.h
> index 4dc961c..3cf01a5 100644
> --- a/drivers/staging/iio/Documentation/iio_utils.h
> +++ b/drivers/staging/iio/Documentation/iio_utils.h
> @@ -319,7 +319,6 @@ inline int build_channel_array(const char *device_dir,
>  			}
>  			fscanf(sysfsfp, "%u", &current->enabled);
>  			fclose(sysfsfp);
> -			free(filename);
>  			current->scale = 1.0;
>  			current->offset = 0;
>  			current->name = strndup(ent->d_name,

Re: [PATCH] IIO: Documentation: generic_buffer example: scan_size must be bytes_per_datum

[Jonathan Cameron]

On 02/15/11 14:46, michael.hennerich@analog.com wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> In case a timestamp is present, scan size returned by size_from_channelarray()
> is not longer valid.
Really?  Shouldn't the timestamp just be picked up as another channel assuming
all the relevant attributes are present (which they should be...)

> However size_from_channelarray() must be still called
> in order to setup locations within the buffer.
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> ---
>  drivers/staging/iio/Documentation/generic_buffer.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/staging/iio/Documentation/generic_buffer.c b/drivers/staging/iio/Documentation/generic_buffer.c
> index 131d9d0..c382452 100644
> --- a/drivers/staging/iio/Documentation/generic_buffer.c
> +++ b/drivers/staging/iio/Documentation/generic_buffer.c
> @@ -226,7 +226,13 @@ int main(int argc, char **argv)
>  	ret = write_sysfs_int("enable", buf_dir_name, 1);
>  	if (ret < 0)
>  		goto error_free_buf_dir_name;
> -	scan_size = size_from_channelarray(infoarray, num_channels);
> +
> +	scan_size = read_sysfs_posint("bytes_per_datum", buf_dir_name);
> +	if (ret < 0)
> +		goto error_free_buf_dir_name;
> +
> +	size_from_channelarray(infoarray, num_channels);
> +
>  	data = malloc(scan_size*buf_len);
>  	if (!data) {
>  		ret = -ENOMEM;

Re: [PATCH] IIO: Documentation: iio_utils: Fix typo

[Jonathan Cameron]

On 02/15/11 14:46, michael.hennerich@analog.com wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> 
It's not an actual bug, but your version is certainly easier to read,
hence the ack. Not worth pushing to stable though and might want to have
a description that makes it clear that it is for readability.

Thanks to helpful denizen of another list for providing the following
succinct argument. As he put it when I queried a**b...

Reference: http://www.cs.man.ac.uk/~pjj/bnf/c_syntax.bnf

There's no "**" token, so "a**d" has to be parsed as "a" "*" "*" "d".

And there's no postfix "*" operator, so "a" "*" ... can only be a
mult_exp. Given it's a mult_exp, the "*" "d" is parsed as a cast_exp,
which inevitably means treating it as a unary_exp consisting of a
unary_operator ("*") followed by an id which, all by itself, qualifies
as a primary_exp, a postfix_exp, a unary_exp and a cast_exp, and is
therefore legal as the rhs of a multi_exp.

> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
>  drivers/staging/iio/Documentation/iio_utils.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/staging/iio/Documentation/iio_utils.h b/drivers/staging/iio/Documentation/iio_utils.h
> index 3cf01a5..4b023aa 100644
> --- a/drivers/staging/iio/Documentation/iio_utils.h
> +++ b/drivers/staging/iio/Documentation/iio_utils.h
> @@ -374,7 +374,7 @@ inline int build_channel_array(const char *device_dir,
>  		}
>  	}
>  	/* reorder so that the array is in index order*/
> -	current = malloc(sizeof(**ci_array)**counter);
> +	current = malloc(sizeof(**ci_array)*(*counter));
>  	if (current == NULL) {
>  		ret = -ENOMEM;
>  		goto error_cleanup_array;

Re: [PATCH] IIO: Documentation: generic_buffer example: scan_size must be bytes_per_datum

[Michael Hennerich]

On 02/22/2011 07:48 PM, Jonathan Cameron wrote:
> On 02/15/11 14:46, michael.hennerich@analog.com wrote:
>   
>> From: Michael Hennerich <michael.hennerich@analog.com>
>>
>> In case a timestamp is present, scan size returned by size_from_channelarray()
>> is not longer valid.
>>     
> Really?  Shouldn't the timestamp just be picked up as another channel assuming
> all the relevant attributes are present (which they should be...)
>   
I think the pronunciation is on 'should be'.
Looks like all my drivers don't do it, as well as some of yours.

My assumption was that bytes_per_datum must be properly set.
And that the timestamp is always assumed to be the last 8bytes aligned
on a 64-bit boundary.
I guess some drivers need fixing.
 
>   
>> However size_from_channelarray() must be still called
>> in order to setup locations within the buffer.
>>
>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
>> ---
>>  drivers/staging/iio/Documentation/generic_buffer.c |    8 +++++++-
>>  1 files changed, 7 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/staging/iio/Documentation/generic_buffer.c b/drivers/staging/iio/Documentation/generic_buffer.c
>> index 131d9d0..c382452 100644
>> --- a/drivers/staging/iio/Documentation/generic_buffer.c
>> +++ b/drivers/staging/iio/Documentation/generic_buffer.c
>> @@ -226,7 +226,13 @@ int main(int argc, char **argv)
>>       ret = write_sysfs_int("enable", buf_dir_name, 1);
>>       if (ret < 0)
>>               goto error_free_buf_dir_name;
>> -     scan_size = size_from_channelarray(infoarray, num_channels);
>> +
>> +     scan_size = read_sysfs_posint("bytes_per_datum", buf_dir_name);
>> +     if (ret < 0)
>> +             goto error_free_buf_dir_name;
>> +
>> +     size_from_channelarray(infoarray, num_channels);
>> +
>>       data = malloc(scan_size*buf_len);
>>       if (!data) {
>>               ret = -ENOMEM;
>>     
>   


-- 
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH] IIO: Documentation: generic_buffer example: scan_size must be bytes_per_datum

[Jonathan Cameron]

On 02/22/11 20:21, Michael Hennerich wrote:
> On 02/22/2011 07:48 PM, Jonathan Cameron wrote:
>> On 02/15/11 14:46, michael.hennerich@analog.com wrote:
>>   
>>> From: Michael Hennerich <michael.hennerich@analog.com>
>>>
>>> In case a timestamp is present, scan size returned by size_from_channelarray()
>>> is not longer valid.
>>>     
>> Really?  Shouldn't the timestamp just be picked up as another channel assuming
>> all the relevant attributes are present (which they should be...)
>>   
> I think the pronunciation is on 'should be'.
> Looks like all my drivers don't do it, as well as some of yours.
> 
> My assumption was that bytes_per_datum must be properly set.
That should certainly be the case.  We could put a sanity check in to
test both numbers are the same as clearly we've gotten it wrong in some
drivers.
> And that the timestamp is always assumed to be the last 8bytes aligned
> on a 64-bit boundary.
It is at the moment but that might change so lets do it right now..
> I guess some drivers need fixing.
Yes. It looks that way. Good thing you picked up on this problem.
>  
>>   
>>> However size_from_channelarray() must be still called
>>> in order to setup locations within the buffer.
>>>
>>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
>>> ---
>>>  drivers/staging/iio/Documentation/generic_buffer.c |    8 +++++++-
>>>  1 files changed, 7 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/staging/iio/Documentation/generic_buffer.c b/drivers/staging/iio/Documentation/generic_buffer.c
>>> index 131d9d0..c382452 100644
>>> --- a/drivers/staging/iio/Documentation/generic_buffer.c
>>> +++ b/drivers/staging/iio/Documentation/generic_buffer.c
>>> @@ -226,7 +226,13 @@ int main(int argc, char **argv)
>>>       ret = write_sysfs_int("enable", buf_dir_name, 1);
>>>       if (ret < 0)
>>>               goto error_free_buf_dir_name;
>>> -     scan_size = size_from_channelarray(infoarray, num_channels);
>>> +
>>> +     scan_size = read_sysfs_posint("bytes_per_datum", buf_dir_name);
>>> +     if (ret < 0)
>>> +             goto error_free_buf_dir_name;
>>> +
>>> +     size_from_channelarray(infoarray, num_channels);
>>> +
>>>       data = malloc(scan_size*buf_len);
>>>       if (!data) {
>>>               ret = -ENOMEM;
>>>     
>>   
> 
>