On Fri, Dec 2, 2011 at 3:37 PM, Johan Hedberg <johan.hedberg@xxxxxxxxx> wrote:
> Hi Bartosz,
>
> On Fri, Dec 02, 2011, Bartosz Szatkowski wrote:
>> +static struct error_code {
>> + const char *name;
>> + guint8 code;
>> +} map_errors[] = {
>> + {"Success", G_OBEX_RSP_SUCCESS},
>> + {"Bad Request", G_OBEX_RSP_BAD_REQUEST},
>> + {"Not Implemented", G_OBEX_RSP_NOT_IMPLEMENTED},
>> + {"Service Unavailable", G_OBEX_RSP_SERVICE_UNAVAILABLE},
>> + {"Forbidden", G_OBEX_RSP_FORBIDDEN},
>> + {"Unauthorized", G_OBEX_RSP_UNAUTHORIZED},
>> + {"Precondition Failed", G_OBEX_RSP_PRECONDITION_FAILED},
>> + {"Not Acceptable", G_OBEX_RSP_NOT_ACCEPTABLE},
>> + {"Not Found", G_OBEX_RSP_NOT_FOUND},
>> + { }
>
> The table is missing spaces on each line after { and before }
>
>> +static const char *get_error_string(guint8 err_code)
>> +{
>> + struct error_code *error;
>> +
>> + for (error = map_errors; error != NULL; error++)
>> + if (error->code == err_code)
>> + return error->name;
>> +
>> + return NULL;
>> +}
>
> It'd probably make sense to make this kind of function more widely
> available, maybe through gobex. However instead of a NULL return I'd
> return "<unknown>" to avoid passing NULL pointers to %s format
> specifiers like you're doing below (I know this will typically just work
> and add a "(null)" to the string, but it's still not a very good
> practice and can cause misleading logs:
>
>> + reply = g_dbus_create_error(map->msg,
>> + "org.openobex.Error.Response",
>> + "%s(0x%X)", get_error_string(err_code),
>> + err_code);
>
> You're also missing a space between %s and ( and please use 0x%02x for
> single byte values.
>
> Johan
OK
--
Pozdrowienia - Cheers,
Bartosz Szatkowski
--
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
