* [PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API
2013-05-16 16:25 [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Anderson Lizardo
@ 2013-05-16 16:25 ` Anderson Lizardo
2013-05-17 5:14 ` [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Marcel Holtmann
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Anderson Lizardo @ 2013-05-16 16:25 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair
2013-05-16 16:25 [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Anderson Lizardo
2013-05-16 16:25 ` [PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API Anderson Lizardo
@ 2013-05-17 5:14 ` Marcel Holtmann
2013-05-17 11:05 ` Anderson Lizardo
2013-05-17 14:31 ` [PATCH BlueZ v2 " Anderson Lizardo
2013-05-21 13:06 ` [PATCH BlueZ v3 " Anderson Lizardo
3 siblings, 1 reply; 14+ messages in thread
From: Marcel Holtmann @ 2013-05-17 5:14 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair
2013-05-17 5:14 ` [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Marcel Holtmann
@ 2013-05-17 11:05 ` Anderson Lizardo
2013-05-17 12:07 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 14+ messages in thread
From: Anderson Lizardo @ 2013-05-17 11:05 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair
2013-05-17 11:05 ` Anderson Lizardo
@ 2013-05-17 12:07 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 14+ messages in thread
From: Luiz Augusto von Dentz @ 2013-05-17 12:07 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: Marcel Holtmann, linux-bluetooth
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair
2013-05-16 16:25 [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Anderson Lizardo
2013-05-16 16:25 ` [PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API Anderson Lizardo
2013-05-17 5:14 ` [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Marcel Holtmann
@ 2013-05-17 14:31 ` Anderson Lizardo
2013-05-17 14:31 ` [PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API Anderson Lizardo
2013-05-20 23:51 ` [PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Johan Hedberg
2013-05-21 13:06 ` [PATCH BlueZ v3 " Anderson Lizardo
3 siblings, 2 replies; 14+ messages in thread
From: Anderson Lizardo @ 2013-05-17 14:31 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API
2013-05-17 14:31 ` [PATCH BlueZ v2 " Anderson Lizardo
@ 2013-05-17 14:31 ` Anderson Lizardo
2013-05-20 23:51 ` [PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Johan Hedberg
1 sibling, 0 replies; 14+ messages in thread
From: Anderson Lizardo @ 2013-05-17 14:31 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair
2013-05-17 14:31 ` [PATCH BlueZ v2 " Anderson Lizardo
2013-05-17 14:31 ` [PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API Anderson Lizardo
@ 2013-05-20 23:51 ` Johan Hedberg
2013-05-21 6:10 ` Marcel Holtmann
2013-05-21 11:02 ` Anderson Lizardo
1 sibling, 2 replies; 14+ messages in thread
From: Johan Hedberg @ 2013-05-20 23:51 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair
2013-05-20 23:51 ` [PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Johan Hedberg
@ 2013-05-21 6:10 ` Marcel Holtmann
2013-05-21 11:02 ` Anderson Lizardo
1 sibling, 0 replies; 14+ messages in thread
From: Marcel Holtmann @ 2013-05-21 6:10 UTC (permalink / raw)
To: Johan Hedberg; +Cc: Anderson Lizardo, linux-bluetooth
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair
2013-05-20 23:51 ` [PATCH BlueZ v2 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Johan Hedberg
2013-05-21 6:10 ` Marcel Holtmann
@ 2013-05-21 11:02 ` Anderson Lizardo
1 sibling, 0 replies; 14+ messages in thread
From: Anderson Lizardo @ 2013-05-21 11:02 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH BlueZ v3 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair
2013-05-16 16:25 [PATCH BlueZ 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Anderson Lizardo
` (2 preceding siblings ...)
2013-05-17 14:31 ` [PATCH BlueZ v2 " Anderson Lizardo
@ 2013-05-21 13:06 ` Anderson Lizardo
2013-05-21 13:06 ` [PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API Anderson Lizardo
2013-05-28 13:25 ` [PATCH BlueZ v3 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Anderson Lizardo
3 siblings, 2 replies; 14+ messages in thread
From: Anderson Lizardo @ 2013-05-21 13:06 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API
2013-05-21 13:06 ` [PATCH BlueZ v3 " Anderson Lizardo
@ 2013-05-21 13:06 ` Anderson Lizardo
2013-05-28 13:25 ` [PATCH BlueZ v3 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Anderson Lizardo
1 sibling, 0 replies; 14+ messages in thread
From: Anderson Lizardo @ 2013-05-21 13:06 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH BlueZ v3 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair
2013-05-21 13:06 ` [PATCH BlueZ v3 " Anderson Lizardo
2013-05-21 13:06 ` [PATCH BlueZ 2/2] core: Avoid unnecessary gboolean on pincode callback API Anderson Lizardo
@ 2013-05-28 13:25 ` Anderson Lizardo
2013-06-13 14:26 ` Johan Hedberg
1 sibling, 1 reply; 14+ messages in thread
From: Anderson Lizardo @ 2013-05-28 13:25 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ v3 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair
2013-05-28 13:25 ` [PATCH BlueZ v3 1/2] plugins: Use open()/read() instead of fopen()/fread() on autopair Anderson Lizardo
@ 2013-06-13 14:26 ` Johan Hedberg
0 siblings, 0 replies; 14+ messages in thread
From: Johan Hedberg @ 2013-06-13 14:26 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
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
^ permalink raw reply [flat|nested] 14+ messages in thread