Hi Luiz, > On Thu, Feb 19, 2015 at 4:42 AM, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > Hi Arman, > > On Thu, Feb 19, 2015 at 2:18 PM, Luiz Augusto von Dentz > <luiz.dentz@xxxxxxxxx> wrote: >> Hi Arman, >> >> On Thu, Feb 19, 2015 at 2:12 AM, Arman Uguray <armansito@xxxxxxxxxxxx> wrote: >>> gatt_db_attribute_write returns false if realloc returns NULL in the >>> case where there is no callback. If the write is performed with a 0 >>> length value and 0 offset, realloc will return NULL according to its >>> specification (as it will practically act as 'free'). This patch fixes >>> the code so that this case isn't treated as an error. >>> --- >>> src/shared/gatt-db.c | 11 +++++------ >>> 1 file changed, 5 insertions(+), 6 deletions(-) >>> >>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c >>> index 3d2e690..b331f54 100644 >>> --- a/src/shared/gatt-db.c >>> +++ b/src/shared/gatt-db.c >>> @@ -1464,7 +1464,7 @@ bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset, >>> } >>> >>> /* Guard against invalid access if offset equals to value length */ >>> - value = offset == attrib->value_len ? NULL : &attrib->value[offset]; >>> + value = attrib->value_len - offset ? &attrib->value[offset] : NULL; >> >> Im not following why you are changing read as well here? Btw, all this >> does is reverse the check so Im not sure how it would make any >> difference. >> Yeah, I guess this one isn't necessary at all, it does the same thing as before. >>> >>> func(attrib, 0, value, attrib->value_len - offset, user_data); >>> >>> @@ -1557,15 +1557,14 @@ bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset, >>> void *buf; >>> >>> buf = realloc(attrib->value, len + offset); >>> - if (!buf) >>> + if (!buf && len + offset > 0) >>> return false; >> >> We should not reach realloc in this case, Id say we return if len == 0 >> since it should really be a nop, for clearing there already exists >> gatt_db_attribute_reset and I don't think it worth complicating more >> the code bellow. >> We reach this code path since we clear the cached value in the writes, but I like your approach better. >>> - attrib->value = buf; >>> - >>> /* Init data in the first allocation */ >>> - if (!attrib->value_len) >>> - memset(attrib->value, 0, offset); >>> + if (!attrib->value) >>> + memset(buf, 0, offset); >>> >>> + attrib->value = buf; >>> attrib->value_len = len + offset; >>> } >>> >>> -- >>> 2.2.0.rc0.207.ga3a616c > > I went ahead applied a fix. > Thanks, I tested your patch and fixes the issue. > > -- > Luiz Augusto von Dentz Thanks! Arman -- 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
