[PATCH] Fixed empty 'N:' parameter handling for VCARDs

[PATCH] Fixed empty 'N:' parameter handling for VCARDs

[Radoslaw Jablonski]

Bluetooth PBAP specification expects for call history listing,
that parameter N: shall be empty when we cannot retrieve personal data
from PSE phone book.
Some devices(by example BH-903) are having problems when after N: parameter
 unnecessary characters occurs. List of dialed/incoming calls on carkit
then is useless - carkit shows only blank lines and it's impossible to
determine who made call.

In previous version unnecessary semicolons were added after N:("N:;;;;")
to represent empty name.
Now if none of the contact fields is available, then adding real empty "N:"
parameter (without semicolons).
---
 plugins/vcard.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/plugins/vcard.c b/plugins/vcard.c
index 5948a4a..8c7147b 100644
--- a/plugins/vcard.c
+++ b/plugins/vcard.c
@@ -123,6 +123,27 @@ static void add_slash(char *dest, const char *src, int len_max, int len)
 	return;
 }
 
+/* checks if there is at least one present contact field with personal data */
+static gboolean contact_fields_present(struct phonebook_contact * contact)
+{
+	if (contact->family && strlen(contact->family))
+		return TRUE;
+
+	if (contact->given && strlen(contact->given))
+		return TRUE;
+	if (contact->additional && strlen(contact->additional))
+		return TRUE;
+
+	if (contact->prefix && strlen(contact->prefix))
+		return TRUE;
+
+	if (contact->suffix && strlen(contact->suffix))
+		return TRUE;
+
+	/* none of the personal data fields is present*/
+	return FALSE;
+}
+
 static void vcard_printf_begin(GString *vcards, uint8_t format)
 {
 	vcard_printf(vcards, "BEGIN:VCARD");
@@ -136,6 +157,12 @@ static void vcard_printf_begin(GString *vcards, uint8_t format)
 static void vcard_printf_name(GString *vcards,
 					struct phonebook_contact *contact)
 {
+	if (contact_fields_present(contact) == FALSE) {
+		/* if all fields are empty we need to have empty N: parameter */
+		vcard_printf(vcards, "N:");
+		return;
+	}
+
 	vcard_printf(vcards, "N:%s;%s;%s;%s;%s", contact->family,
 				contact->given, contact->additional,
 				contact->prefix, contact->suffix);
-- 
1.6.0.4

Re: [PATCH] Fixed empty 'N:' parameter handling for VCARDs

[Marcel Holtmann]

Hi Radek,

> Bluetooth PBAP specification expects for call history listing,
> that parameter N: shall be empty when we cannot retrieve personal data
> from PSE phone book.
> Some devices(by example BH-903) are having problems when after N: parameter
>  unnecessary characters occurs. List of dialed/incoming calls on carkit
> then is useless - carkit shows only blank lines and it's impossible to
> determine who made call.
> 
> In previous version unnecessary semicolons were added after N:("N:;;;;")
> to represent empty name.
> Now if none of the contact fields is available, then adding real empty "N:"
> parameter (without semicolons).
> ---
>  plugins/vcard.c |   27 +++++++++++++++++++++++++++
>  1 files changed, 27 insertions(+), 0 deletions(-)
> 
> diff --git a/plugins/vcard.c b/plugins/vcard.c
> index 5948a4a..8c7147b 100644
> --- a/plugins/vcard.c
> +++ b/plugins/vcard.c
> @@ -123,6 +123,27 @@ static void add_slash(char *dest, const char *src, int len_max, int len)
>  	return;
>  }
>  
> +/* checks if there is at least one present contact field with personal data */
> +static gboolean contact_fields_present(struct phonebook_contact * contact)
> +{
> +	if (contact->family && strlen(contact->family))
> +		return TRUE;

I asked to use strlen(...) > 0 as test here. Please fix this.

> +
> +	if (contact->given && strlen(contact->given))
> +		return TRUE;
> +	if (contact->additional && strlen(contact->additional))
> +		return TRUE;

Why are these two tight together. Use an extra empty line.

> +	if (contact->prefix && strlen(contact->prefix))
> +		return TRUE;
> +
> +	if (contact->suffix && strlen(contact->suffix))
> +		return TRUE;
> +
> +	/* none of the personal data fields is present*/
> +	return FALSE;
> +}
> +
>  static void vcard_printf_begin(GString *vcards, uint8_t format)
>  {
>  	vcard_printf(vcards, "BEGIN:VCARD");
> @@ -136,6 +157,12 @@ static void vcard_printf_begin(GString *vcards, uint8_t format)
>  static void vcard_printf_name(GString *vcards,
>  					struct phonebook_contact *contact)
>  {
> +	if (contact_fields_present(contact) == FALSE) {
> +		/* if all fields are empty we need to have empty N: parameter */
> +		vcard_printf(vcards, "N:");

I would be good to add a bit more extra text here why we are doing this.
You should mention the broken headset and what is expect outcome.

Also please sort the contact_fields_resent() function about
vcard_printf_name to keep them close together.

Regards

Marcel

[PATCH] Fixed empty 'N:' parameter handling for VCARDs

[Radoslaw Jablonski]

Bluetooth PBAP specification expects for call history listing,
that parameter N: shall be empty when we cannot retrieve personal data
from PSE phone book.
Some devices(by example BH-903) are having problems when after N: parameter
 unnecessary characters occurs. List of dialed/incoming calls on carkit
then is useless - carkit shows only blank lines and it's impossible to
determine who made call.

In previous version unnecessary semicolons were added after N:("N:;;;;")
to represent empty name.
Now if none of the contact fields is available, then adding real empty "N:"
parameter (without semicolons).
---
 plugins/vcard.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/plugins/vcard.c b/plugins/vcard.c
index 5948a4a..108f6bd 100644
--- a/plugins/vcard.c
+++ b/plugins/vcard.c
@@ -133,9 +133,44 @@ static void vcard_printf_begin(GString *vcards, uint8_t format)
 		vcard_printf(vcards, "VERSION:2.1");
 }
 
+/* checks if there is at least one present contact field with personal data */
+static gboolean contact_fields_present(struct phonebook_contact * contact)
+{
+	if (contact->family && strlen(contact->family) > 0)
+		return TRUE;
+
+	if (contact->given && strlen(contact->given) > 0)
+		return TRUE;
+
+	if (contact->additional && strlen(contact->additional) > 0)
+		return TRUE;
+
+	if (contact->prefix && strlen(contact->prefix) > 0)
+		return TRUE;
+
+	if (contact->suffix && strlen(contact->suffix) > 0)
+		return TRUE;
+
+	/* none of the personal data fields is present*/
+	return FALSE;
+}
+
 static void vcard_printf_name(GString *vcards,
 					struct phonebook_contact *contact)
 {
+	if (contact_fields_present(contact) == FALSE) {
+		/*
+		 * If fields are empty, then add only 'N:' as parameter.
+		 * This is crucial for some devices ( Nokia BH-903) which
+		 * have problems with history listings - can't determine
+		 * if parameter is really empty if there are unnecessary
+		 * characters after'N:' (like 'N:;;;;').
+		 * We need to add only'N:' param - without semicolons.
+		 */
+		vcard_printf(vcards, "N:");
+		return;
+	}
+
 	vcard_printf(vcards, "N:%s;%s;%s;%s;%s", contact->family,
 				contact->given, contact->additional,
 				contact->prefix, contact->suffix);
-- 
1.6.0.4

Re: [PATCH] Fixed empty 'N:' parameter handling for VCARDs

[Marcel Holtmann]

Hi Radek,

> Bluetooth PBAP specification expects for call history listing,
> that parameter N: shall be empty when we cannot retrieve personal data
> from PSE phone book.
> Some devices(by example BH-903) are having problems when after N: parameter
>  unnecessary characters occurs. List of dialed/incoming calls on carkit
> then is useless - carkit shows only blank lines and it's impossible to
> determine who made call.

please be a bit consistent with your spaces here.

> In previous version unnecessary semicolons were added after N:("N:;;;;")
> to represent empty name.
> Now if none of the contact fields is available, then adding real empty "N:"
> parameter (without semicolons).
> ---
>  plugins/vcard.c |   35 +++++++++++++++++++++++++++++++++++
>  1 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/plugins/vcard.c b/plugins/vcard.c
> index 5948a4a..108f6bd 100644
> --- a/plugins/vcard.c
> +++ b/plugins/vcard.c
> @@ -133,9 +133,44 @@ static void vcard_printf_begin(GString *vcards, uint8_t format)
>  		vcard_printf(vcards, "VERSION:2.1");
>  }
>  
> +/* checks if there is at least one present contact field with personal data */

"check if there is at least one contact field with personal data
present"

> +static gboolean contact_fields_present(struct phonebook_contact * contact)
> +{
> +	if (contact->family && strlen(contact->family) > 0)
> +		return TRUE;
> +
> +	if (contact->given && strlen(contact->given) > 0)
> +		return TRUE;
> +
> +	if (contact->additional && strlen(contact->additional) > 0)
> +		return TRUE;
> +
> +	if (contact->prefix && strlen(contact->prefix) > 0)
> +		return TRUE;
> +
> +	if (contact->suffix && strlen(contact->suffix) > 0)
> +		return TRUE;
> +
> +	/* none of the personal data fields is present*/

"none of the personal data fields are present"

> +	return FALSE;
> +}
> +
>  static void vcard_printf_name(GString *vcards,
>  					struct phonebook_contact *contact)
>  {
> +	if (contact_fields_present(contact) == FALSE) {
> +		/*
> +		 * If fields are empty, then add only 'N:' as parameter.
> +		 * This is crucial for some devices ( Nokia BH-903) which

Please be consistent with spaces.

> +		 * have problems with history listings - can't determine
> +		 * if parameter is really empty if there are unnecessary
> +		 * characters after'N:' (like 'N:;;;;').
> +		 * We need to add only'N:' param - without semicolons.

Same here.

> +		 */
> +		vcard_printf(vcards, "N:");
> +		return;
> +	}
> +
>  	vcard_printf(vcards, "N:%s;%s;%s;%s;%s", contact->family,
>  				contact->given, contact->additional,
>  				contact->prefix, contact->suffix);

Regards

Marcel

Re: [PATCH] Fixed empty 'N:' parameter handling for VCARDs

[Johan Hedberg]

Hi Marcel,

On Fri, Jul 09, 2010, Marcel Holtmann wrote:
> > Bluetooth PBAP specification expects for call history listing,
> > that parameter N: shall be empty when we cannot retrieve personal data
> > from PSE phone book.
> > Some devices(by example BH-903) are having problems when after N: parameter
> >  unnecessary characters occurs. List of dialed/incoming calls on carkit
> > then is useless - carkit shows only blank lines and it's impossible to
> > determine who made call.
> 
> please be a bit consistent with your spaces here.
> 
> > In previous version unnecessary semicolons were added after N:("N:;;;;")
> > to represent empty name.
> > Now if none of the contact fields is available, then adding real empty "N:"
> > parameter (without semicolons).
> > ---
> >  plugins/vcard.c |   35 +++++++++++++++++++++++++++++++++++
> >  1 files changed, 35 insertions(+), 0 deletions(-)
> > 
> > diff --git a/plugins/vcard.c b/plugins/vcard.c
> > index 5948a4a..108f6bd 100644
> > --- a/plugins/vcard.c
> > +++ b/plugins/vcard.c
> > @@ -133,9 +133,44 @@ static void vcard_printf_begin(GString *vcards, uint8_t format)
> >  		vcard_printf(vcards, "VERSION:2.1");
> >  }
> >  
> > +/* checks if there is at least one present contact field with personal data */
> 
> "check if there is at least one contact field with personal data
> present"
> 
> > +static gboolean contact_fields_present(struct phonebook_contact * contact)
> > +{
> > +	if (contact->family && strlen(contact->family) > 0)
> > +		return TRUE;
> > +
> > +	if (contact->given && strlen(contact->given) > 0)
> > +		return TRUE;
> > +
> > +	if (contact->additional && strlen(contact->additional) > 0)
> > +		return TRUE;
> > +
> > +	if (contact->prefix && strlen(contact->prefix) > 0)
> > +		return TRUE;
> > +
> > +	if (contact->suffix && strlen(contact->suffix) > 0)
> > +		return TRUE;
> > +
> > +	/* none of the personal data fields is present*/
> 
> "none of the personal data fields are present"
> 
> > +	return FALSE;
> > +}
> > +
> >  static void vcard_printf_name(GString *vcards,
> >  					struct phonebook_contact *contact)
> >  {
> > +	if (contact_fields_present(contact) == FALSE) {
> > +		/*
> > +		 * If fields are empty, then add only 'N:' as parameter.
> > +		 * This is crucial for some devices ( Nokia BH-903) which
> 
> Please be consistent with spaces.
> 
> > +		 * have problems with history listings - can't determine
> > +		 * if parameter is really empty if there are unnecessary
> > +		 * characters after'N:' (like 'N:;;;;').
> > +		 * We need to add only'N:' param - without semicolons.
> 
> Same here.

I went ahead and fixed these issues myself so we can get another obexd
release out. The fixed patch is now upstream.

Johan

[PATCH] Fixed empty 'N:' parameter handling for VCARDs

[Radoslaw Jablonski]

Bluetooth PBAP specification expects for call history listing, that parameter N: shall be empty when we cannot retrieve personal data from PSE phone book.
Some devices(by example BH-903) are having problems when after N: parameter unnecessary characters occurs. List of dialed/incoming calls on carkit then is useless - carkit shows only blank lines and it's impossible to determine who made call.

In previous version unnecessary semicolons were added after N: parameter ("N:;;;;") to represent empty name.
Now if none of the contact fields is available, then adding real empty "N:" parameter (without semicolons).
---
 plugins/vcard.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/plugins/vcard.c b/plugins/vcard.c
index 5948a4a..8c7147b 100644
--- a/plugins/vcard.c
+++ b/plugins/vcard.c
@@ -123,6 +123,27 @@ static void add_slash(char *dest, const char *src, int len_max, int len)
 	return;
 }
 
+/* checks if there is at least one present contact field with personal data */
+static gboolean contact_fields_present(struct phonebook_contact * contact)
+{
+	if (contact->family && strlen(contact->family))
+		return TRUE;
+
+	if (contact->given && strlen(contact->given))
+		return TRUE;
+	if (contact->additional && strlen(contact->additional))
+		return TRUE;
+
+	if (contact->prefix && strlen(contact->prefix))
+		return TRUE;
+
+	if (contact->suffix && strlen(contact->suffix))
+		return TRUE;
+
+	/* none of the personal data fields is present*/
+	return FALSE;
+}
+
 static void vcard_printf_begin(GString *vcards, uint8_t format)
 {
 	vcard_printf(vcards, "BEGIN:VCARD");
@@ -136,6 +157,12 @@ static void vcard_printf_begin(GString *vcards, uint8_t format)
 static void vcard_printf_name(GString *vcards,
 					struct phonebook_contact *contact)
 {
+	if (contact_fields_present(contact) == FALSE) {
+		/* if all fields are empty we need to have empty N: parameter */
+		vcard_printf(vcards, "N:");
+		return;
+	}
+
 	vcard_printf(vcards, "N:%s;%s;%s;%s;%s", contact->family,
 				contact->given, contact->additional,
 				contact->prefix, contact->suffix);
-- 
1.6.0.4

[PATCH] Fixed empty 'N:' parameter handling for VCARDs

[ext-jablonski.radoslaw]

Hi!
I've created patch problems with history listing with BH-903.. 
After fix everything works as described in PBAP spec and problem no longer occurs.
	
Patch is available on my gitorius master branch clone for obexd-packaging: 
git://gitorious.org/~jradosla/maemo-bluetooth/jradoslas-obexd-packaging.git 
It is located in:  debian/patches/006-Fixed-empty-N-parameter-handling-for-VCARDs.patch

BR,
Radek

Re: [PATCH] Fixed empty 'N:' parameter handling for VCARDs

[Marcel Holtmann]

Hi Radek,

> I've created patch problems with history listing with BH-903.. 
> After fix everything works as described in PBAP spec and problem no longer occurs.
> 	
> Patch is available on my gitorius master branch clone for obexd-packaging: 
> git://gitorious.org/~jradosla/maemo-bluetooth/jradoslas-obexd-packaging.git 
> It is located in:  debian/patches/006-Fixed-empty-N-parameter-handling-for-VCARDs.patch

yeah, that is not how this works. Take the patch and send it to the
mailing list for review. I am not going to extract a patch from your
packaging repos nor is anybody else.

Regards

Marcel