Hi Johan.
On Thu, Apr 26, 2012 at 10:31 AM, Johan Hedberg <johan.hedberg@xxxxxxxxx> wrote:
> Hi Jefferson,
>
> On Fri, Apr 13, 2012, Jefferson Delfes wrote:
>> --- a/attrib/gatt.c
>> +++ b/attrib/gatt.c
>> @@ -47,6 +47,22 @@ struct discover_primary {
>> void *user_data;
>> };
>>
>> +struct find_included {
>> + GAttrib *attrib;
>> + uint16_t end;
>> + GSList *includes;
>> + gatt_cb_t cb;
>> + void *user_data;
>> + uint16_t missing_uuids; /* number of missing 128-bit UUID */
>> + gboolean final; /* final data flag */
>> + unsigned int err;
>> +};
>> +
>> +struct find_include_data {
>> + struct find_included *find_incl;
>> + struct gatt_included *gatt_incl;
>> +};
>
> The reason I didn't apply this immediately and instead put it aside (and
> then forgot about it) is that I'm having trouble figuring out these data
> structures, their purposes and their relation to each other. The first
> thing that starts off the confusion is that you've named than based on
> an action instead of something that could be considered an object.
>
> So please try to come up with better naming for them, and if the new
> names do not in themselves already explain most of the purpose then at
> least provide proper code comments to clarify this.
Hmm, ok. I will think up a better naming for them.
> Another thing that threw me off is the "fid" variable naming. The
> existing and most common use of "id" is a shorthand for "identifier". So
> what's "fid"? A file identifier? Please come up with better names for
> these things. In many cases the context is quite clear and if you can't
> come up with anything better simply "data" could be good enough.
Yeah, it seems ambiguous.
> Johan
Thanks for your feedback.
--
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
