[PATCH 1/1] Add DBG() calls to authorization logic

[PATCH 1/1] Add DBG() calls to authorization logic

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

Sometimes BlueZ Agent is disappear suddenly or not registered
because of agent problem. but Bluez just prints "Operation Not
permiited"

This will help debugging.
---
 audio/manager.c |    2 +-
 src/adapter.c   |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/audio/manager.c b/audio/manager.c
index aa2547c..20453e6 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -604,7 +604,7 @@ static void hf_io_cb(GIOChannel *chan, gpointer data)
 	perr = audio_device_request_authorization(device, server_uuid,
 						gateway_auth_cb, device);
 	if (perr < 0) {
-		DBG("Authorization denied!");
+		DBG("Authorization denied: %s", strerror(-perr));
 		gateway_set_state(device, GATEWAY_STATE_DISCONNECTED);
 	}
 
diff --git a/src/adapter.c b/src/adapter.c
index d10db10..15d502f 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3107,6 +3107,7 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
 
 	agent = device_get_agent(device);
 	if (!agent) {
+		DBG("Can't find device agent");
 		g_free(auth);
 		return -EPERM;
 	}
-- 
1.7.5.4

Re: [PATCH 1/1] Add DBG() calls to authorization logic

[Marcel Holtmann]

Hi Chan-yeol,

> Sometimes BlueZ Agent is disappear suddenly or not registered
> because of agent problem. but Bluez just prints "Operation Not
> permiited"
> 
> This will help debugging.
> ---
>  audio/manager.c |    2 +-
>  src/adapter.c   |    1 +
>  2 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/audio/manager.c b/audio/manager.c
> index aa2547c..20453e6 100644
> --- a/audio/manager.c
> +++ b/audio/manager.c
> @@ -604,7 +604,7 @@ static void hf_io_cb(GIOChannel *chan, gpointer data)
>  	perr = audio_device_request_authorization(device, server_uuid,
>  						gateway_auth_cb, device);
>  	if (perr < 0) {
> -		DBG("Authorization denied!");
> +		DBG("Authorization denied: %s", strerror(-perr));
>  		gateway_set_state(device, GATEWAY_STATE_DISCONNECTED);
>  	}

it is fine to keep this as debug messages, ...

>  
> diff --git a/src/adapter.c b/src/adapter.c
> index d10db10..15d502f 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -3107,6 +3107,7 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
>  
>  	agent = device_get_agent(device);
>  	if (!agent) {
> +		DBG("Can't find device agent");

..., but this debug message is kinda useless as debug message. It should
be a warning message. Having to enable debug to see is a broken idea.

>  		g_free(auth);
>  		return -EPERM;
>  	}

Regards

Marcel

[PATCH 1/1] Add DBG() calls to authorization logic

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

Sometimes BlueZ Agent is disappear suddenly or not registered
because of agent problem. but Bluez just prints "Operation Not
permiited"

This will help debugging.

As Marcel guided, I replace DBG with error because I can't find how to
print out warning message
---
 audio/manager.c |    2 +-
 src/adapter.c   |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/audio/manager.c b/audio/manager.c
index aa2547c..20453e6 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -604,7 +604,7 @@ static void hf_io_cb(GIOChannel *chan, gpointer data)
 	perr = audio_device_request_authorization(device, server_uuid,
 						gateway_auth_cb, device);
 	if (perr < 0) {
-		DBG("Authorization denied!");
+		DBG("Authorization denied: %s", strerror(-perr));
 		gateway_set_state(device, GATEWAY_STATE_DISCONNECTED);
 	}
 
diff --git a/src/adapter.c b/src/adapter.c
index d10db10..7b83392 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3107,6 +3107,7 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
 
 	agent = device_get_agent(device);
 	if (!agent) {
+		error("Can't find device agent");
 		g_free(auth);
 		return -EPERM;
 	}
-- 
1.7.5.4

Re: [PATCH 1/1] Add DBG() calls to authorization logic

[Marcel Holtmann]

Hi Chan-yeol,

> Sometimes BlueZ Agent is disappear suddenly or not registered
> because of agent problem. but Bluez just prints "Operation Not
> permiited"
> 
> This will help debugging.

> As Marcel guided, I replace DBG with error because I can't find how to
> print out warning message

comments like this that are addressed for the reviewer, should come
after the ---

> ---
>  audio/manager.c |    2 +-
>  src/adapter.c   |    1 +
>  2 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/audio/manager.c b/audio/manager.c
> index aa2547c..20453e6 100644
> --- a/audio/manager.c
> +++ b/audio/manager.c
> @@ -604,7 +604,7 @@ static void hf_io_cb(GIOChannel *chan, gpointer data)
>  	perr = audio_device_request_authorization(device, server_uuid,
>  						gateway_auth_cb, device);
>  	if (perr < 0) {
> -		DBG("Authorization denied!");
> +		DBG("Authorization denied: %s", strerror(-perr));
>  		gateway_set_state(device, GATEWAY_STATE_DISCONNECTED);
>  	}
>  
> diff --git a/src/adapter.c b/src/adapter.c
> index d10db10..7b83392 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -3107,6 +3107,7 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
>  
>  	agent = device_get_agent(device);
>  	if (!agent) {
> +		error("Can't find device agent");

It is not really an error actually. So lets use a warning here. Since
warn() is not present you need to implement that one first.

>  		g_free(auth);
>  		return -EPERM;
>  	}

Regards

Marcel

[PATCH 1/2] Add function for printing warnings

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

---
 src/log.c |   12 ++++++++++++
 src/log.h |    1 +
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/src/log.c b/src/log.c
index 4a50117..adbfba9 100644
--- a/src/log.c
+++ b/src/log.c
@@ -55,6 +55,18 @@ void error(const char *format, ...)
 	va_end(ap);
 }
 
+void warn(const char *format, ...)
+{
+	va_list ap;
+
+	va_start(ap, format);
+
+	vsyslog(LOG_WARNING, format, ap);
+
+	va_end(ap);
+}
+
+
 void btd_debug(const char *format, ...)
 {
 	va_list ap;
diff --git a/src/log.h b/src/log.h
index 51c52e6..cdcb146 100644
--- a/src/log.h
+++ b/src/log.h
@@ -23,6 +23,7 @@
 
 void info(const char *format, ...) __attribute__((format(printf, 1, 2)));
 void error(const char *format, ...) __attribute__((format(printf, 1, 2)));
+void warn(const char *format, ...) __attribute__((format(printf, 1, 2)));
 
 void btd_debug(const char *format, ...) __attribute__((format(printf, 1, 2)));
 
-- 
1.7.5.4

[PATCH 2/2] Add warning if there is no authorization agent

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

Sometimes BlueZ Agent is disappear suddenly or not registered
because of agent problem. but Bluez just prints "Operation Not
permiited"

This will help debugging.
---
 audio/manager.c |    2 +-
 src/adapter.c   |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/audio/manager.c b/audio/manager.c
index aa2547c..20453e6 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -604,7 +604,7 @@ static void hf_io_cb(GIOChannel *chan, gpointer data)
 	perr = audio_device_request_authorization(device, server_uuid,
 						gateway_auth_cb, device);
 	if (perr < 0) {
-		DBG("Authorization denied!");
+		DBG("Authorization denied: %s", strerror(-perr));
 		gateway_set_state(device, GATEWAY_STATE_DISCONNECTED);
 	}
 
diff --git a/src/adapter.c b/src/adapter.c
index d10db10..a5bab34 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3107,6 +3107,7 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
 
 	agent = device_get_agent(device);
 	if (!agent) {
+		warn("Can't find device agent");
 		g_free(auth);
 		return -EPERM;
 	}
-- 
1.7.5.4

Re: [PATCH 1/2] Add function for printing warnings

[Marcel Holtmann]

Hi Chan-yeol,

> ---
>  src/log.c |   12 ++++++++++++
>  src/log.h |    1 +
>  2 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/src/log.c b/src/log.c
> index 4a50117..adbfba9 100644
> --- a/src/log.c
> +++ b/src/log.c
> @@ -55,6 +55,18 @@ void error(const char *format, ...)
>  	va_end(ap);
>  }
>  
> +void warn(const char *format, ...)
> +{
> +	va_list ap;
> +
> +	va_start(ap, format);
> +
> +	vsyslog(LOG_WARNING, format, ap);
> +
> +	va_end(ap);
> +}
> +
> +

the patch is fine, but just for consistence sake, you should put warn()
after info() function.

And no double empty lines between functions please. That is not part of
our coding style. And if you saw it somewhere else in the code, then
that is an oversight.

>  void btd_debug(const char *format, ...)
>  {
>  	va_list ap;
> diff --git a/src/log.h b/src/log.h
> index 51c52e6..cdcb146 100644
> --- a/src/log.h
> +++ b/src/log.h
> @@ -23,6 +23,7 @@
>  
>  void info(const char *format, ...) __attribute__((format(printf, 1, 2)));
>  void error(const char *format, ...) __attribute__((format(printf, 1, 2)));
> +void warn(const char *format, ...) __attribute__((format(printf, 1, 2)));

Same here. warn() comes after info().

>  
>  void btd_debug(const char *format, ...) __attribute__((format(printf, 1, 2)));
>  

Regards

Marcel

[PATCH v2 1/3] src: Add function for printing warnings

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

---
 src/log.c |   11 +++++++++++
 src/log.h |    1 +
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/src/log.c b/src/log.c
index 4a50117..75a98a9 100644
--- a/src/log.c
+++ b/src/log.c
@@ -44,6 +44,17 @@ void info(const char *format, ...)
 	va_end(ap);
 }
 
+void warn(const char *format, ...)
+{
+	va_list ap;
+
+	va_start(ap, format);
+
+	vsyslog(LOG_WARNING, format, ap);
+
+	va_end(ap);
+}
+
 void error(const char *format, ...)
 {
 	va_list ap;
diff --git a/src/log.h b/src/log.h
index 51c52e6..3d34fa3 100644
--- a/src/log.h
+++ b/src/log.h
@@ -22,6 +22,7 @@
  */
 
 void info(const char *format, ...) __attribute__((format(printf, 1, 2)));
+void warn(const char *format, ...) __attribute__((format(printf, 1, 2)));
 void error(const char *format, ...) __attribute__((format(printf, 1, 2)));
 
 void btd_debug(const char *format, ...) __attribute__((format(printf, 1, 2)));
-- 
1.7.5.4

[PATCH v2 2/3] adapter: Add warning if there is no authorization agent

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

Sometimes BlueZ Agent is disappear suddenly or not registered
because of agent problem. but Bluez just prints "Operation Not
permiited"

This will help debugging.
---
 src/adapter.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index d10db10..a5bab34 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3107,6 +3107,7 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
 
 	agent = device_get_agent(device);
 	if (!agent) {
+		warn("Can't find device agent");
 		g_free(auth);
 		return -EPERM;
 	}
-- 
1.7.5.4

[PATCH v2 3/3] audio: Add missing debug message in authorization

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

---
 audio/manager.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/audio/manager.c b/audio/manager.c
index aa2547c..20453e6 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -604,7 +604,7 @@ static void hf_io_cb(GIOChannel *chan, gpointer data)
 	perr = audio_device_request_authorization(device, server_uuid,
 						gateway_auth_cb, device);
 	if (perr < 0) {
-		DBG("Authorization denied!");
+		DBG("Authorization denied: %s", strerror(-perr));
 		gateway_set_state(device, GATEWAY_STATE_DISCONNECTED);
 	}
 
-- 
1.7.5.4

Re: [PATCH v2 1/3] src: Add function for printing warnings

[Johan Hedberg]

Hi Chan-yeol,

On Mon, Apr 16, 2012, chanyeol.park@samsung.com wrote:
> From: Chan-yeol Park <chanyeol.park@samsung.com>
> 
> ---
>  src/log.c |   11 +++++++++++
>  src/log.h |    1 +
>  2 files changed, 12 insertions(+), 0 deletions(-)

All three patches have been applied. Thanks.

Johan