2012-12-22 15:12:39

by Joao Paulo Rechi Vita

Subject: [PATCH BlueZ 1/4] hog: Fix message text and level when failing to load suspend plugin

---
profiles/input/hog.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/profiles/input/hog.c b/profiles/input/hog.c
index b6ac61d..a9018f5 100644
--- a/profiles/input/hog.c
+++ b/profiles/input/hog.c
@@ -885,7 +885,8 @@ static int hog_init(void)

err = suspend_init(suspend_callback, resume_callback);
if (err < 0)
- DBG("Suspend: %s(%d)", strerror(-err), -err);
+ error("Loading suspend plugin failed: %s (%d)", strerror(-err),
+ -err);
else
suspend_supported = TRUE;

--
1.7.11.7

2012-12-22 19:56:00

by Marcel Holtmann

Subject: Re: [PATCH BlueZ 4/4] hog: Remove pre-existing suspend FIFO

Hi Joao Paulo,

> >> If bluetoothd crashes the exit routine of the suspend plugin will not be
> >> executed, leaving the suspend FIFO behind and preventing the plugin load
> >> on subsequent executions. This commit checks for pre-existence of the
> >> suspend FIFO and tries to remove and re-create it.
> >> ---
> >> profiles/input/suspend-dummy.c | 16 ++++++++++++++++
> >> 1 file changed, 16 insertions(+)
> >>
> >> diff --git a/profiles/input/suspend-dummy.c b/profiles/input/suspend-dummy.c
> >> index 33b790a..43384c0 100644
> >> --- a/profiles/input/suspend-dummy.c
> >> +++ b/profiles/input/suspend-dummy.c
> >> @@ -119,6 +119,22 @@ int suspend_init(suspend_event suspend, resume_event resume)
> >>
> >> if (mkfifo(HOG_SUSPEND_FIFO, S_IRWXU) < 0) {
> >> int err = -errno;
> >> +
> >> + if (err == -EEXIST) {
> >> + DBG("FIFO (%s) already exists, trying to remove",
> >> + HOG_SUSPEND_FIFO);
> >> +
> >> + /* remove pre-existing FIFO and retry */
> >> + if (remove(HOG_SUSPEND_FIFO) < 0) {
> >
> > you are looking for unlink() to use here.
> >
>
> What's the problem with remove()? From the manpage it's part of
> stdio.h and calls unlink() for files, and rmdir() for directories.
> From my understanding if someone else created a directory on the FIFO
> path, unlink() will return with EISDIR and we would need to handle
> this error as well.

if it accidentally has been a directory with that name, then I want to
see a big fat error. Not some magic to make it work.

Regards

Marcel

2012-12-22 18:41:02

by Joao Paulo Rechi Vita

Subject: Re: [PATCH BlueZ 4/4] hog: Remove pre-existing suspend FIFO

On Sat, Dec 22, 2012 at 3:39 PM, Joao Paulo Rechi Vita
<[email protected]> wrote:
> On Sat, Dec 22, 2012 at 1:36 PM, Marcel Holtmann <[email protected]> wr=
ote:
>> Hi Joao Paulo,
>>
>>> If bluetoothd crashes the exit routine of the suspend plugin will not b=
e
>>> executed, leaving the suspend FIFO behind and preventing the plugin loa=
d
>>> on subsequent executions. This commit checks for pre-existence of the
>>> suspend FIFO and tries to remove and re-create it.
>>> ---
>>> profiles/input/suspend-dummy.c | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/profiles/input/suspend-dummy.c b/profiles/input/suspend-du=
mmy.c
>>> index 33b790a..43384c0 100644
>>> --- a/profiles/input/suspend-dummy.c
>>> +++ b/profiles/input/suspend-dummy.c
>>> @@ -119,6 +119,22 @@ int suspend_init(suspend_event suspend, resume_eve=
nt resume)
>>>
>>> if (mkfifo(HOG_SUSPEND_FIFO, S_IRWXU) < 0) {
>>> int err =3D -errno;
>>> +
>>> + if (err =3D=3D -EEXIST) {
>>> + DBG("FIFO (%s) already exists, trying to remove",
>>> + HOG_SUSPEND_FIFO)=
;
>>> +
>>> + /* remove pre-existing FIFO and retry */
>>> + if (remove(HOG_SUSPEND_FIFO) < 0) {
>>
>> you are looking for unlink() to use here.
>>
>
> What's the problem with remove()? From the manpage it's part of
> stdio.h and calls unlink() for files, and rmdir() for directories.
> From my understanding if someone else created a directory on the FIFO
> path, unlink() will return with EISDIR and we would need to handle
> this error as well.
>

And BTW, remove() is also been used on suspend_exit().

--
Jo=C3=A3o Paulo Rechi Vita
Openbossa Labs - INdT

2012-12-22 18:39:11

by Joao Paulo Rechi Vita

Subject: Re: [PATCH BlueZ 4/4] hog: Remove pre-existing suspend FIFO

On Sat, Dec 22, 2012 at 1:36 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Joao Paulo,
>
>> If bluetoothd crashes the exit routine of the suspend plugin will not be
>> executed, leaving the suspend FIFO behind and preventing the plugin load
>> on subsequent executions. This commit checks for pre-existence of the
>> suspend FIFO and tries to remove and re-create it.
>> ---
>> profiles/input/suspend-dummy.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/profiles/input/suspend-dummy.c b/profiles/input/suspend-dum=
my.c
>> index 33b790a..43384c0 100644
>> --- a/profiles/input/suspend-dummy.c
>> +++ b/profiles/input/suspend-dummy.c
>> @@ -119,6 +119,22 @@ int suspend_init(suspend_event suspend, resume_even=
t resume)
>>
>> if (mkfifo(HOG_SUSPEND_FIFO, S_IRWXU) < 0) {
>> int err =3D -errno;
>> +
>> + if (err =3D=3D -EEXIST) {
>> + DBG("FIFO (%s) already exists, trying to remove",
>> + HOG_SUSPEND_FIFO);
>> +
>> + /* remove pre-existing FIFO and retry */
>> + if (remove(HOG_SUSPEND_FIFO) < 0) {
>
> you are looking for unlink() to use here.
>

What's the problem with remove()? From the manpage it's part of
stdio.h and calls unlink() for files, and rmdir() for directories.
>From my understanding if someone else created a directory on the FIFO
path, unlink() will return with EISDIR and we would need to handle
this error as well.

--
Jo=C3=A3o Paulo Rechi Vita
Openbossa Labs - INdT

2012-12-22 16:51:23

by Johan Hedberg

Subject: Re: [PATCH BlueZ 4/4] hog: Remove pre-existing suspend FIFO

Hi Marcel,

On Sat, Dec 22, 2012, Marcel Holtmann wrote:
> > + if (remove(HOG_SUSPEND_FIFO) < 0) {
>
> you are looking for unlink() to use here.

Since I already applied the patches I went ahead and fixed this myself.

Johan

2012-12-22 16:36:11

by Marcel Holtmann

Subject: Re: [PATCH BlueZ 4/4] hog: Remove pre-existing suspend FIFO

Hi Joao Paulo,

> If bluetoothd crashes the exit routine of the suspend plugin will not be
> executed, leaving the suspend FIFO behind and preventing the plugin load
> on subsequent executions. This commit checks for pre-existence of the
> suspend FIFO and tries to remove and re-create it.
> ---
> profiles/input/suspend-dummy.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/profiles/input/suspend-dummy.c b/profiles/input/suspend-dummy.c
> index 33b790a..43384c0 100644
> --- a/profiles/input/suspend-dummy.c
> +++ b/profiles/input/suspend-dummy.c
> @@ -119,6 +119,22 @@ int suspend_init(suspend_event suspend, resume_event resume)
>
> if (mkfifo(HOG_SUSPEND_FIFO, S_IRWXU) < 0) {
> int err = -errno;
> +
> + if (err == -EEXIST) {
> + DBG("FIFO (%s) already exists, trying to remove",
> + HOG_SUSPEND_FIFO);
> +
> + /* remove pre-existing FIFO and retry */
> + if (remove(HOG_SUSPEND_FIFO) < 0) {

you are looking for unlink() to use here.

> + err = -errno;
> + error("Failed to remove FIFO (%s): %s (%d)",
> + HOG_SUSPEND_FIFO, strerror(-err), -err);
> + return err;
> + }
> +
> + return suspend_init(suspend, resume);
> + }
> +
> error("Can't create FIFO (%s): %s (%d)", HOG_SUSPEND_FIFO,
> strerror(-err), -err);
> return err;

Regards

Marcel

2012-12-22 16:35:11

by Johan Hedberg

Subject: Re: [PATCH BlueZ 1/4] hog: Fix message text and level when failing to load suspend plugin

Hi Jo?o Paulo,

On Sat, Dec 22, 2012, Jo?o Paulo Rechi Vita wrote:
> ---
> profiles/input/hog.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)

All four patches have been applied. Thanks.

Johan

2012-12-22 15:12:42

by Joao Paulo Rechi Vita

Subject: [PATCH BlueZ 4/4] hog: Remove pre-existing suspend FIFO

If bluetoothd crashes the exit routine of the suspend plugin will not be
executed, leaving the suspend FIFO behind and preventing the plugin load
on subsequent executions. This commit checks for pre-existence of the
suspend FIFO and tries to remove and re-create it.
---
profiles/input/suspend-dummy.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/profiles/input/suspend-dummy.c b/profiles/input/suspend-dummy.c
index 33b790a..43384c0 100644
--- a/profiles/input/suspend-dummy.c
+++ b/profiles/input/suspend-dummy.c
@@ -119,6 +119,22 @@ int suspend_init(suspend_event suspend, resume_event resume)

if (mkfifo(HOG_SUSPEND_FIFO, S_IRWXU) < 0) {
int err = -errno;
+
+ if (err == -EEXIST) {
+ DBG("FIFO (%s) already exists, trying to remove",
+ HOG_SUSPEND_FIFO);
+
+ /* remove pre-existing FIFO and retry */
+ if (remove(HOG_SUSPEND_FIFO) < 0) {
+ err = -errno;
+ error("Failed to remove FIFO (%s): %s (%d)",
+ HOG_SUSPEND_FIFO, strerror(-err), -err);
+ return err;
+ }
+
+ return suspend_init(suspend, resume);
+ }
+
error("Can't create FIFO (%s): %s (%d)", HOG_SUSPEND_FIFO,
strerror(-err), -err);
return err;
--
1.7.11.7

2012-12-22 15:12:41

by Joao Paulo Rechi Vita

Subject: [PATCH BlueZ 3/4] hog: Fix error message formating

---
profiles/input/suspend-dummy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/input/suspend-dummy.c b/profiles/input/suspend-dummy.c
index 14aabd0..33b790a 100644
--- a/profiles/input/suspend-dummy.c
+++ b/profiles/input/suspend-dummy.c
@@ -119,7 +119,7 @@ int suspend_init(suspend_event suspend, resume_event resume)

if (mkfifo(HOG_SUSPEND_FIFO, S_IRWXU) < 0) {
int err = -errno;
- error("Can't create FIFO (%s) : %s(%d)", HOG_SUSPEND_FIFO,
+ error("Can't create FIFO (%s): %s (%d)", HOG_SUSPEND_FIFO,
strerror(-err), -err);
return err;
}
--
1.7.11.7

2012-12-22 15:12:40

by Joao Paulo Rechi Vita

Subject: [PATCH BlueZ 2/4] hog: Add debug info when initializing the suspend plugin

---
profiles/input/suspend-dummy.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/profiles/input/suspend-dummy.c b/profiles/input/suspend-dummy.c
index b43946d..14aabd0 100644
--- a/profiles/input/suspend-dummy.c
+++ b/profiles/input/suspend-dummy.c
@@ -124,6 +124,8 @@ int suspend_init(suspend_event suspend, resume_event resume)
return err;
}

+ DBG("Created suspend-dummy FIFO on %s", HOG_SUSPEND_FIFO);
+
ret = fifo_open();
if (ret < 0)
remove(HOG_SUSPEND_FIFO);
--
1.7.11.7