On Wed, Jun 15, 2011 at 3:39 PM, Johan Hedberg <johan.hedberg@xxxxxxxxx> wrote: > Hi Luiz, > > On Wed, Jun 15, 2011, Luiz Augusto von Dentz wrote: >> So this while loop is a bit dangerous since it is supposing to be >> doing io, so what about separating the read of apparam and body in two >> callbacks, the apparam could probably be consider a metadata of you >> are actually accessing so separating those might be a good idea in the >> long term. If so then read should probably only read the data itself >> and never mix with apparams, then we have a e.g. read_apparams which >> is only called once before reading the actual data, how about that? The loop is only used before we start streaming. That is because if there was no streaming started, request will be finished after returning from OpenOBEX callback. To still allow plugins to use the original approach, the loop guarantees that there will be another read() if the service plugin did not explicitly returned -EAGAIN, so read() still decides explicitly when it wants the suspending to happen, therefore it is perfectly aware of when to call obex_object_set_io_flags(). This is just how OpenOBEX works - unless we are streaming body headers, we should add headers when getting event and suspend if we want to postpone finishing the request. I would not call this loop "dangerous". > Seems more or less ok with me, but I'd call it get_apparams since it's > not really a stream of data like the body. FWIW, I've completely missed > the extension of the read callback to return other types of data besides > that which is contained within body headers. The original intent of it > was to only operate on the actual object body and it'd be nice if we > could get it back to that state again. Well, we have also another possible header types and generally in some near future a profile may need to use more than just application parameters header and/or body headers (e.g. user defined headers). This might also allow sending Length header asynchronously (which by quick looking at code is not possible now). With approach I presented the cost of changes is very low and additionally makes it extremely easy to return all possible headers. Also everything just works, together with support for asynchronous I/O. Adding additional callback will unnecessarily complicate the code. -- Slawomir Bochenski -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
