[PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[Anderson Lizardo]

open()/read() is more common on BlueZ code. Incidentally, get rid of
this compilation error (using gcc 4.6.3):

plugins/autopair.c: In function ‘autopair_init’:
plugins/autopair.c:154:8: error: ignoring return value of ‘fread’,
declared with attribute warn_unused_result [-Werror=unused-result]
---

WARNING: compilation tested only.

 plugins/autopair.c |   20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/plugins/autopair.c b/plugins/autopair.c
index 5d90f9d..63f906d 100644
--- a/plugins/autopair.c
+++ b/plugins/autopair.c
@@ -27,6 +27,8 @@
 
 #include <stdbool.h>
 #include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
 
 #include <bluetooth/bluetooth.h>
 #include <glib.h>
@@ -146,15 +148,19 @@ static struct btd_adapter_driver autopair_driver = {
 static int autopair_init(void)
 {
 	/* Initialize the random seed from /dev/urandom */
-	unsigned int seed = time(NULL);
-	FILE *f;
-
-	f = fopen("/dev/urandom", "rb");
-	if (f != NULL) {
-		fread(&seed, sizeof(seed), 1, f);
-		fclose(f);
+	unsigned int seed;
+	ssize_t nread = 0;
+	int fd;
+
+	fd = open("/dev/urandom", O_RDONLY);
+	if (fd != -1) {
+		nread = read(fd, &seed, sizeof(seed));
+		close(fd);
 	}
 
+	if (nread < (ssize_t) sizeof(seed))
+		seed = time(NULL);
+
 	srand(seed);
 
 	return btd_register_adapter_driver(&autopair_driver);
-- 
1.7.9.5

[PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API

[Anderson Lizardo]

Use standard "bool" type instead. Callers in plugins/* are fixed on the
same commit to avoid compilation errors.
---
 plugins/autopair.c |    8 ++++----
 plugins/wiimote.c  |    3 ++-
 src/adapter.c      |    5 ++---
 src/adapter.h      |    2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/plugins/autopair.c b/plugins/autopair.c
index 63f906d..63c3400 100644
--- a/plugins/autopair.c
+++ b/plugins/autopair.c
@@ -51,9 +51,9 @@
  */
 
 static ssize_t autopair_pincb(struct btd_adapter *adapter,
-					struct btd_device *device,
-					char *pinbuf, gboolean *display,
-					unsigned int attempt)
+						struct btd_device *device,
+						char *pinbuf, bool *display,
+						unsigned int attempt)
 {
 	char addr[18];
 	char pinstr[7];
@@ -109,7 +109,7 @@ static ssize_t autopair_pincb(struct btd_adapter *adapter,
 
 			snprintf(pinstr, sizeof(pinstr), "%06d",
 						rand() % 1000000);
-			*display = TRUE;
+			*display = true;
 			memcpy(pinbuf, pinstr, 6);
 			return 6;
 
diff --git a/plugins/wiimote.c b/plugins/wiimote.c
index 12312b6..beda307 100644
--- a/plugins/wiimote.c
+++ b/plugins/wiimote.c
@@ -70,7 +70,8 @@ static const char *wii_names[] = {
 };
 
 static ssize_t wii_pincb(struct btd_adapter *adapter, struct btd_device *device,
-			char *pinbuf, gboolean *display, unsigned int attempt)
+						char *pinbuf, bool *display,
+						unsigned int attempt)
 {
 	uint16_t vendor, product;
 	char addr[18], name[25];
diff --git a/src/adapter.c b/src/adapter.c
index adb2a17..73e75a2 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -4810,8 +4810,7 @@ static ssize_t btd_adapter_pin_cb_iter_next(
 					struct btd_adapter_pin_cb_iter *iter,
 					struct btd_adapter *adapter,
 					struct btd_device *device,
-					char *pin_buf,
-					gboolean *display)
+					char *pin_buf, bool *display)
 {
 	btd_adapter_pin_cb_t cb;
 	ssize_t ret;
@@ -4836,7 +4835,7 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
 	const struct mgmt_ev_pin_code_request *ev = param;
 	struct btd_adapter *adapter = user_data;
 	struct btd_device *device;
-	gboolean display = FALSE;
+	bool display = false;
 	char pin[17];
 	ssize_t pinlen;
 	char addr[18];
diff --git a/src/adapter.h b/src/adapter.h
index 2de8730..32b12c0 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -132,7 +132,7 @@ int btd_cancel_authorization(guint id);
 int btd_adapter_restore_powered(struct btd_adapter *adapter);
 
 typedef ssize_t (*btd_adapter_pin_cb_t) (struct btd_adapter *adapter,
-			struct btd_device *dev, char *out, gboolean *display,
+			struct btd_device *dev, char *out, bool *display,
 							unsigned int attempt);
 void btd_adapter_register_pin_cb(struct btd_adapter *adapter,
 						btd_adapter_pin_cb_t cb);
-- 
1.7.9.5

Re: [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[Marcel Holtmann]

Hi Anderson,

> open()/read() is more common on BlueZ code. Incidentally, get rid of
> this compilation error (using gcc 4.6.3):
> 
> plugins/autopair.c: In function ‘autopair_init’:
> plugins/autopair.c:154:8: error: ignoring return value of ‘fread’,
> declared with attribute warn_unused_result [-Werror=unused-result]
> ---
> 
> WARNING: compilation tested only.
> 
> plugins/autopair.c |   20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/plugins/autopair.c b/plugins/autopair.c
> index 5d90f9d..63f906d 100644
> --- a/plugins/autopair.c
> +++ b/plugins/autopair.c
> @@ -27,6 +27,8 @@
> 
> #include <stdbool.h>
> #include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> 
> #include <bluetooth/bluetooth.h>
> #include <glib.h>
> @@ -146,15 +148,19 @@ static struct btd_adapter_driver autopair_driver = {
> static int autopair_init(void)
> {
> 	/* Initialize the random seed from /dev/urandom */
> -	unsigned int seed = time(NULL);
> -	FILE *f;
> -
> -	f = fopen("/dev/urandom", "rb");
> -	if (f != NULL) {
> -		fread(&seed, sizeof(seed), 1, f);
> -		fclose(f);
> +	unsigned int seed;
> +	ssize_t nread = 0;
> +	int fd;
> +
> +	fd = open("/dev/urandom", O_RDONLY);
> +	if (fd != -1) {

we are still checking this with fd < 0.

> +		nread = read(fd, &seed, sizeof(seed));
> +		close(fd);
> 	}

	else
		nread = -1;

I really dislike initialized variable if there is no reason for doing that.

> 
> +	if (nread < (ssize_t) sizeof(seed))
> +		seed = time(NULL);
> +
> 	srand(seed);

Also I wonder why this is all so complicated.

	if (fd < 0) {
		int n;

		n = read(fd, ...);
		if (n < sizeof(...))
			seed = time();

		close(fd);
	} else
		seed = time();

Regards

Marcel

Re: [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[Anderson Lizardo]

Hi Marcel,

On Fri, May 17, 2013 at 1:14 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Also I wonder why this is all so complicated.
>
>         if (fd < 0) {
>                 int n;
>
>                 n = read(fd, ...);
>                 if (n < sizeof(...))
>                         seed = time();
>
>                 close(fd);
>         } else
>                 seed = time();

I intentionally avoided nested if()'s and the duplicated "seed =
time(...)" assignment because it seemed to actually make it more
complex IMHO. I will change to the version above, but I don't see why
it is more readable than my v2 (plus the changes you proposed above).
My main concern for this specific code is still to get the build fixed
:)

Note that the cast is necessary because "sizeof()" is unsigned.

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[Luiz Augusto von Dentz]

Hi Anderson,

On Fri, May 17, 2013 at 2:05 PM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi Marcel,
>
> On Fri, May 17, 2013 at 1:14 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> Also I wonder why this is all so complicated.
>>
>>         if (fd < 0) {
>>                 int n;
>>
>>                 n = read(fd, ...);
>>                 if (n < sizeof(...))
>>                         seed = time();
>>
>>                 close(fd);
>>         } else
>>                 seed = time();
>
> I intentionally avoided nested if()'s and the duplicated "seed =
> time(...)" assignment because it seemed to actually make it more
> complex IMHO. I will change to the version above, but I don't see why
> it is more readable than my v2 (plus the changes you proposed above).
> My main concern for this specific code is still to get the build fixed
> :)

You can always create a helper function if the code gets too complicated.


--
Luiz Augusto von Dentz

[PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[Anderson Lizardo]

open()/read() is more common on BlueZ code. Incidentally, get rid of
this compilation error (using gcc 4.6.3):

plugins/autopair.c: In function ‘autopair_init’:
plugins/autopair.c:154:8: error: ignoring return value of ‘fread’,
declared with attribute warn_unused_result [-Werror=unused-result]
---

v2: Incorporate comments from Marcel.

 plugins/autopair.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/plugins/autopair.c b/plugins/autopair.c
index 5d90f9d..7e71efa 100644
--- a/plugins/autopair.c
+++ b/plugins/autopair.c
@@ -27,6 +27,8 @@
 
 #include <stdbool.h>
 #include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
 
 #include <bluetooth/bluetooth.h>
 #include <glib.h>
@@ -146,14 +148,20 @@ static struct btd_adapter_driver autopair_driver = {
 static int autopair_init(void)
 {
 	/* Initialize the random seed from /dev/urandom */
-	unsigned int seed = time(NULL);
-	FILE *f;
+	unsigned int seed;
+	int fd;
 
-	f = fopen("/dev/urandom", "rb");
-	if (f != NULL) {
-		fread(&seed, sizeof(seed), 1, f);
-		fclose(f);
-	}
+	fd = open("/dev/urandom", O_RDONLY);
+	if (fd < 0) {
+		ssize_t n;
+
+		n = read(fd, &seed, sizeof(seed));
+		if (n < (ssize_t) sizeof(seed))
+			seed = time(NULL);
+
+		close(fd);
+	} else
+		seed = time(NULL);
 
 	srand(seed);
 
-- 
1.7.9.5

[PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API

[Anderson Lizardo]

Use standard "bool" type instead. Callers in plugins/* are fixed on the
same commit to avoid compilation errors.
---
 plugins/autopair.c |    8 ++++----
 plugins/wiimote.c  |    3 ++-
 src/adapter.c      |    5 ++---
 src/adapter.h      |    2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/plugins/autopair.c b/plugins/autopair.c
index 7e71efa..98d9a90 100644
--- a/plugins/autopair.c
+++ b/plugins/autopair.c
@@ -51,9 +51,9 @@
  */
 
 static ssize_t autopair_pincb(struct btd_adapter *adapter,
-					struct btd_device *device,
-					char *pinbuf, gboolean *display,
-					unsigned int attempt)
+						struct btd_device *device,
+						char *pinbuf, bool *display,
+						unsigned int attempt)
 {
 	char addr[18];
 	char pinstr[7];
@@ -109,7 +109,7 @@ static ssize_t autopair_pincb(struct btd_adapter *adapter,
 
 			snprintf(pinstr, sizeof(pinstr), "%06d",
 						rand() % 1000000);
-			*display = TRUE;
+			*display = true;
 			memcpy(pinbuf, pinstr, 6);
 			return 6;
 
diff --git a/plugins/wiimote.c b/plugins/wiimote.c
index 12312b6..beda307 100644
--- a/plugins/wiimote.c
+++ b/plugins/wiimote.c
@@ -70,7 +70,8 @@ static const char *wii_names[] = {
 };
 
 static ssize_t wii_pincb(struct btd_adapter *adapter, struct btd_device *device,
-			char *pinbuf, gboolean *display, unsigned int attempt)
+						char *pinbuf, bool *display,
+						unsigned int attempt)
 {
 	uint16_t vendor, product;
 	char addr[18], name[25];
diff --git a/src/adapter.c b/src/adapter.c
index adb2a17..73e75a2 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -4810,8 +4810,7 @@ static ssize_t btd_adapter_pin_cb_iter_next(
 					struct btd_adapter_pin_cb_iter *iter,
 					struct btd_adapter *adapter,
 					struct btd_device *device,
-					char *pin_buf,
-					gboolean *display)
+					char *pin_buf, bool *display)
 {
 	btd_adapter_pin_cb_t cb;
 	ssize_t ret;
@@ -4836,7 +4835,7 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
 	const struct mgmt_ev_pin_code_request *ev = param;
 	struct btd_adapter *adapter = user_data;
 	struct btd_device *device;
-	gboolean display = FALSE;
+	bool display = false;
 	char pin[17];
 	ssize_t pinlen;
 	char addr[18];
diff --git a/src/adapter.h b/src/adapter.h
index 2de8730..32b12c0 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -132,7 +132,7 @@ int btd_cancel_authorization(guint id);
 int btd_adapter_restore_powered(struct btd_adapter *adapter);
 
 typedef ssize_t (*btd_adapter_pin_cb_t) (struct btd_adapter *adapter,
-			struct btd_device *dev, char *out, gboolean *display,
+			struct btd_device *dev, char *out, bool *display,
 							unsigned int attempt);
 void btd_adapter_register_pin_cb(struct btd_adapter *adapter,
 						btd_adapter_pin_cb_t cb);
-- 
1.7.9.5

Re: [PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[Johan Hedberg]

Hi Lizardo,

On Fri, May 17, 2013, Anderson Lizardo wrote:
> +	fd = open("/dev/urandom", O_RDONLY);
> +	if (fd < 0) {
> +		ssize_t n;
> +
> +		n = read(fd, &seed, sizeof(seed));
> +		if (n < (ssize_t) sizeof(seed))
> +			seed = time(NULL);
> +
> +		close(fd);
> +	} else
> +		seed = time(NULL);

So, it's not always wise to go blindly copying code examples/suggestions
from Marcel without thinking a bit. You're checking for invalid fd when
you should be checking for a valid one (>= 0).

Johan

Re: [PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[Marcel Holtmann]

Hi Johan,

>> +	fd = open("/dev/urandom", O_RDONLY);
>> +	if (fd < 0) {
>> +		ssize_t n;
>> +
>> +		n = read(fd, &seed, sizeof(seed));
>> +		if (n < (ssize_t) sizeof(seed))
>> +			seed = time(NULL);
>> +
>> +		close(fd);
>> +	} else
>> +		seed = time(NULL);
> 
> So, it's not always wise to go blindly copying code examples/suggestions
> from Marcel without thinking a bit. You're checking for invalid fd when
> you should be checking for a valid one (>= 0).

good one.

Having to read this again, can someone please tell when a Linux platform will not have /dev/urandom and we really need to care? Wouldn't it be better to bail out loudly with an error message and do not use any auto pair feature at all.

Regards

Marcel

Re: [PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[Anderson Lizardo]

Hi Johan,

On Mon, May 20, 2013 at 7:51 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lizardo,
>
> On Fri, May 17, 2013, Anderson Lizardo wrote:
>> +     fd = open("/dev/urandom", O_RDONLY);
>> +     if (fd < 0) {
>> +             ssize_t n;
>> +
>> +             n = read(fd, &seed, sizeof(seed));
>> +             if (n < (ssize_t) sizeof(seed))
>> +                     seed = time(NULL);
>> +
>> +             close(fd);
>> +     } else
>> +             seed = time(NULL);
>
> So, it's not always wise to go blindly copying code examples/suggestions
> from Marcel without thinking a bit. You're checking for invalid fd when
> you should be checking for a valid one (>= 0).

Sorry about letting this pass. I actually reviewed the snippet, but
was induced to some "braino" which now seems obvious.

I'll try to be more careful, specially given I could not test this
change (as I mentioned on the original patch, but not on this one).

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

[PATCH BlueZ v3 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[Anderson Lizardo]

open()/read() is more common on BlueZ code. Incidentally, get rid of
this compilation error (using gcc 4.6.3):

plugins/autopair.c: In function ‘autopair_init’:
plugins/autopair.c:154:8: error: ignoring return value of ‘fread’,
declared with attribute warn_unused_result [-Werror=unused-result]
---
 plugins/autopair.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

v3:

* Fix buggy logic
* The obvious code path is tested by just starting bluetoothd, which will call
  autopair_init()
* Maybe overkill, but I used this mockup code to test other unreachable code
  paths: http://ix.io/5JC
* I agree with Marcel that we should just fail if /dev/urandom is not readable.
  Otherwise, we are introducing code that will 99% of the time not be run
  (unless someone confirms that we have systems with unusable /dev/urandom).

diff --git a/plugins/autopair.c b/plugins/autopair.c
index 5d90f9d..5aa80df 100644
--- a/plugins/autopair.c
+++ b/plugins/autopair.c
@@ -27,6 +27,8 @@
 
 #include <stdbool.h>
 #include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
 
 #include <bluetooth/bluetooth.h>
 #include <glib.h>
@@ -146,14 +148,20 @@ static struct btd_adapter_driver autopair_driver = {
 static int autopair_init(void)
 {
 	/* Initialize the random seed from /dev/urandom */
-	unsigned int seed = time(NULL);
-	FILE *f;
+	unsigned int seed;
+	int fd;
 
-	f = fopen("/dev/urandom", "rb");
-	if (f != NULL) {
-		fread(&seed, sizeof(seed), 1, f);
-		fclose(f);
-	}
+	fd = open("/dev/urandom", O_RDONLY);
+	if (fd >= 0) {
+		ssize_t n;
+
+		n = read(fd, &seed, sizeof(seed));
+		if (n < (ssize_t) sizeof(seed))
+			seed = time(NULL);
+
+		close(fd);
+	} else
+		seed = time(NULL);
 
 	srand(seed);
 
-- 
1.7.9.5

[PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API

[Anderson Lizardo]

Use standard "bool" type instead. Callers in plugins/* are fixed on the
same commit to avoid compilation errors.
---
 plugins/autopair.c |    8 ++++----
 plugins/wiimote.c  |    3 ++-
 src/adapter.c      |    5 ++---
 src/adapter.h      |    2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/plugins/autopair.c b/plugins/autopair.c
index 5aa80df..e6e5035 100644
--- a/plugins/autopair.c
+++ b/plugins/autopair.c
@@ -51,9 +51,9 @@
  */
 
 static ssize_t autopair_pincb(struct btd_adapter *adapter,
-					struct btd_device *device,
-					char *pinbuf, gboolean *display,
-					unsigned int attempt)
+						struct btd_device *device,
+						char *pinbuf, bool *display,
+						unsigned int attempt)
 {
 	char addr[18];
 	char pinstr[7];
@@ -109,7 +109,7 @@ static ssize_t autopair_pincb(struct btd_adapter *adapter,
 
 			snprintf(pinstr, sizeof(pinstr), "%06d",
 						rand() % 1000000);
-			*display = TRUE;
+			*display = true;
 			memcpy(pinbuf, pinstr, 6);
 			return 6;
 
diff --git a/plugins/wiimote.c b/plugins/wiimote.c
index 12312b6..beda307 100644
--- a/plugins/wiimote.c
+++ b/plugins/wiimote.c
@@ -70,7 +70,8 @@ static const char *wii_names[] = {
 };
 
 static ssize_t wii_pincb(struct btd_adapter *adapter, struct btd_device *device,
-			char *pinbuf, gboolean *display, unsigned int attempt)
+						char *pinbuf, bool *display,
+						unsigned int attempt)
 {
 	uint16_t vendor, product;
 	char addr[18], name[25];
diff --git a/src/adapter.c b/src/adapter.c
index adb2a17..73e75a2 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -4810,8 +4810,7 @@ static ssize_t btd_adapter_pin_cb_iter_next(
 					struct btd_adapter_pin_cb_iter *iter,
 					struct btd_adapter *adapter,
 					struct btd_device *device,
-					char *pin_buf,
-					gboolean *display)
+					char *pin_buf, bool *display)
 {
 	btd_adapter_pin_cb_t cb;
 	ssize_t ret;
@@ -4836,7 +4835,7 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
 	const struct mgmt_ev_pin_code_request *ev = param;
 	struct btd_adapter *adapter = user_data;
 	struct btd_device *device;
-	gboolean display = FALSE;
+	bool display = false;
 	char pin[17];
 	ssize_t pinlen;
 	char addr[18];
diff --git a/src/adapter.h b/src/adapter.h
index 2de8730..32b12c0 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -132,7 +132,7 @@ int btd_cancel_authorization(guint id);
 int btd_adapter_restore_powered(struct btd_adapter *adapter);
 
 typedef ssize_t (*btd_adapter_pin_cb_t) (struct btd_adapter *adapter,
-			struct btd_device *dev, char *out, gboolean *display,
+			struct btd_device *dev, char *out, bool *display,
 							unsigned int attempt);
 void btd_adapter_register_pin_cb(struct btd_adapter *adapter,
 						btd_adapter_pin_cb_t cb);
-- 
1.7.9.5

Re: [PATCH BlueZ v3 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[Anderson Lizardo]

Hi,

On Tue, May 21, 2013 at 9:06 AM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> open()/read() is more common on BlueZ code. Incidentally, get rid of
> this compilation error (using gcc 4.6.3):
>
> plugins/autopair.c: In function ‘autopair_init’:
> plugins/autopair.c:154:8: error: ignoring return value of ‘fread’,
> declared with attribute warn_unused_result [-Werror=unused-result]
> ---
>  plugins/autopair.c |   22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> v3:
>
> * Fix buggy logic
> * The obvious code path is tested by just starting bluetoothd, which will call
>   autopair_init()
> * Maybe overkill, but I used this mockup code to test other unreachable code
>   paths: http://ix.io/5JC
> * I agree with Marcel that we should just fail if /dev/urandom is not readable.
>   Otherwise, we are introducing code that will 99% of the time not be run
>   (unless someone confirms that we have systems with unusable /dev/urandom).

Ping. There were no further comments on the idea of removing the
"time(NULL)" fallback suggested by Marcel.

Should I proceed with dropping the time(NULL) fallback or may this
patch be applied as is?

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH BlueZ v3 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair

[Johan Hedberg]

Hi Lizardo,

On Tue, May 28, 2013, Anderson Lizardo wrote:
> On Tue, May 21, 2013 at 9:06 AM, Anderson Lizardo
> <anderson.lizardo@openbossa.org> wrote:
> > open()/read() is more common on BlueZ code. Incidentally, get rid of
> > this compilation error (using gcc 4.6.3):
> >
> > plugins/autopair.c: In function ‘autopair_init’:
> > plugins/autopair.c:154:8: error: ignoring return value of ‘fread’,
> > declared with attribute warn_unused_result [-Werror=unused-result]
> > ---
> >  plugins/autopair.c |   22 +++++++++++++++-------
> >  1 file changed, 15 insertions(+), 7 deletions(-)
> >
> > v3:
> >
> > * Fix buggy logic
> > * The obvious code path is tested by just starting bluetoothd, which will call
> >   autopair_init()
> > * Maybe overkill, but I used this mockup code to test other unreachable code
> >   paths: http://ix.io/5JC
> > * I agree with Marcel that we should just fail if /dev/urandom is not readable.
> >   Otherwise, we are introducing code that will 99% of the time not be run
> >   (unless someone confirms that we have systems with unusable /dev/urandom).
> 
> Ping. There were no further comments on the idea of removing the
> "time(NULL)" fallback suggested by Marcel.
> 
> Should I proceed with dropping the time(NULL) fallback or may this
> patch be applied as is?

I pushed the current patches, however I wont object to a third one to
get rid of the time(NULL) call.

Johan