Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 16 Jun 2006 10:27:22 -0700
From:      Maksim Yevmenkin <maksim.yevmenkin@savvis.net>
To:        Iain Hibbert <plunky@rya-online.net>
Cc:        freebsd-bluetooth@freebsd.org
Subject:   Re: SDP
Message-ID:  <4492E9FA.2030708@savvis.net>
In-Reply-To: <1150322381.757072.1713.nullmailer@galant.ukfsn.org>
References:  <1150200307.649295.228.nullmailer@galant.ukfsn.org> <44904A60.6080105@savvis.net> <1150322381.757072.1713.nullmailer@galant.ukfsn.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Iain,

>> when this comes up, i always refer people for openssl's err(3)
> 
> Well, I looked at that and I'm not sure what the point of all that is..
> seems very complex for what it is, but maybe some complexity is warranted
> when you have a threaded application.

what i'm trying to say is

1) mixing errno-style and custom style error reporting is, imo, a bad 
idea. doing so usually result in complicated and confusing, imo, code. 
for this reason, i usually choose one style for a particular piece of 
code and stick with it.

2) given a choice between errno-style and custom style error reporting, 
i usually choose custom style *with* reusing errno values for error 
codes. this way custom error reporting is equivalent to "glorified 
errno", but in the same time i have the flexibility to completely 
replace errno values with something else if needed. this approach is 
something learned from personal experience and it worked for me many times.

3) i believe that data encapsulation and reentrant, thread-safe api 
design are good things. for this reason session-based api is used in 
libsdp(3). i choose to save error codes inside the session because it 
fits this model. i'm not saying that is the only right way to do things. 
i'm just trying to explain how i came to write this code.

> When I write code, my ideal would preferably be 'simplicity and elegance'
> and I think in this case, you went too far and it just got complicated for
> no great reason..

perhaps. sdp protocol and data structures are flexible. it is possible 
to slice and dice the data any way one wants. since bluetooth 
specification is a living thing and keeps changing, i opt out for more 
complicated design.

>> there is, of course, little problem. because the errno values are reused, in
>> some cases, it is not possible to say if syscall has failed or something
>> inside library has failed.
> 
> there does not (imo) need to be any such distinction in reality - the
> application program gets a notice of failure on a specific function (by a
> bad return code), and what the failure is. It doesnt matter where the
> error was. eg if sdp_open() returns ENOMEM then you know there was some
> memory failure. It is not important if it was sdp_open() that failed to
> allocate the handle structure, or if it was the kernel that failed to
> allocate the PCB. Of course, you as the library author are responsible to
> make sure that you dont have bugs that cause error returns with valid
> application arguments.

i disagree with you here. in order to troubleshoot any problem one needs 
to know what went wrong and why. it just saves lots of time down the 
road (imo).

i appreciate your comments and respect your position. so, lets just 
agree to disagree here :) whether you choose to use libsdp(3) or not, i 
personally will respect and support your choice. i'm sure others will do 
the same. both implementations already have quite a few differences, so 
adding one more should not be a big issue :)

i think, you do have a point about sdpd(8) profile registration and 
libsdp(3) attribute parsing. i will try to address these (and if anyone 
wants to help please feel free to jump in). and, of course, please feel 
free to continue to ask questions, submit patches etc. its all good stuff :)

>>> specification is not clear at all.  I thought of some way, possibly
>>> extending the attrlist that you used for sdp_search() to specify values
>>> that could be automatically extracted and leaving all the parsing inside
>>> the library.
>> well, here is what i think. sdp(3) client knows *exactly* what it is looking
>> for. usually it is only required to get a few (less than 3) attributes to get
>> all the information.
> 
> btw, I wonder why you made the attrlist take a range instead of an
> attribute? It leaves us with this kind of thing..
> 
> /* List of the attributes we are looking for */
> static uint32_t	attrs[] =
> {
> 	SDP_ATTR_RANGE(	SDP_ATTR_SERVICE_RECORD_HANDLE,
> 			SDP_ATTR_SERVICE_RECORD_HANDLE),
> 	SDP_ATTR_RANGE(	SDP_ATTR_SERVICE_CLASS_ID_LIST,
> 			SDP_ATTR_SERVICE_CLASS_ID_LIST),
> 	SDP_ATTR_RANGE(	SDP_ATTR_PROTOCOL_DESCRIPTOR_LIST,
> 			SDP_ATTR_PROTOCOL_DESCRIPTOR_LIST),
> 	SDP_ATTR_RANGE(	SDP_ATTR_BLUETOOTH_PROFILE_DESCRIPTOR_LIST,
> 			SDP_ATTR_BLUETOOTH_PROFILE_DESCRIPTOR_LIST)
> };
> 
> .. though I'm not sure how often how often a range of attributes could be
> desireable .. in fact, this could be cleared with another macro like:
> 
> #define SDP_ATTR(attr)	SDP_ATTR_RANGE(attr, attr)
> 
> [later] actually, I'm guessing this is related to the spec somehow -
> looking at the BlueZ sdp_lib.h include file which seems to be the
> documentation, they look to use the same concept.

yes. attributes can be request in two ways

1) as individual uint16_t attribute id

2) as uint32_t attribute range (lo, hi)

in order to simplify the code i decided to always use ranges even if one 
attribute is requested. this creates slightly bigger sdp pdu's but (imo) 
makes api simpler. the SDP_ATTR(attr) you suggesting is fine with me.

>> sdp(3) client knows *exactly* what the format of the
>> attributes should be. so, sdp(3) library should provide simple api to locate
>> given element of given type in raw data and extract data from it. imo, more
>> intelligent parsing could require sdp(3) to actually know about the profile
>> client is trying use.
> 
> Yes was thinking along those lines.. the client would provide a structure
> detailing how to parse the required attributes where to put the values.

may be. so far i have not seen a need for that.

thanks,
max



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4492E9FA.2030708>