On Wed, Nov 27, 2013 at 3:09 PM, Mika Kuoppala
<mika.kuoppala@xxxxxxxxxxxxxxx> wrote:
> Daniel Vetter <daniel.vetter@xxxxxxxx> writes:
>> It leads to a big mess when stuff interleaves. Especially with the new
>> patch I've submitted for the drm core to no longer artificially split
>> up debug messages.
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/intel_sdvo.c | 55 ++++++++++++++++++++++++++-------------
>> 1 file changed, 37 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
>> index a583e8f718a7..e88ad95df08f 100644
>> --- a/drivers/gpu/drm/i915/intel_sdvo.c
>> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
>> @@ -413,23 +413,34 @@ static const struct _sdvo_cmd_name {
>> static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
>> const void *args, int args_len)
>> {
>> - int i;
>> + int i, pos = 0;
>> +#define BUF_LEN 256
>> + char buffer[BUF_LEN];
>> +
>> +#define BUF_PRINT(args...) \
>> + pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos - 1, 0), args)
>> +
>
> You want scnprintf here as it returns the true number of bytes written
> instead of what would have been written. Also the size argument includes
> the the trailing null space. Consider:
Actually the semantics of snprintf which only returns the actual
characters written without the terminating 0 at the end is what I want
- the next BUF_PRINT should overwrite things.
In case of a buffer overrunt the max should make sure that we don't
actually overwrite anything, and the BUG_ON below makes sure that the
string we feed to printk is still 0-terminated (hence the - 1 there).
> BUG_ON((pos += scnprintf(buffer + pos, max_t(int, BUF_LEN - pos, 0), args)) >= BUF_LEN)
>
>> - DRM_DEBUG_KMS("%s: W: %02X ",
>> - SDVO_NAME(intel_sdvo), cmd);
>> - for (i = 0; i < args_len; i++)
>> - DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
>> - for (; i < 8; i++)
>> - DRM_LOG_KMS(" ");
>> + for (i = 0; i < args_len; i++) {
>> + BUF_PRINT("%02X ", ((u8 *)args)[i]);
>> + }
>> + for (; i < 8; i++) {
>> + BUF_PRINT(" ");
>> + }
>> for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
>> if (cmd == sdvo_cmd_names[i].cmd) {
>> - DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
>> + BUF_PRINT("(%s)", sdvo_cmd_names[i].name);
>> break;
>> }
>> }
>> - if (i == ARRAY_SIZE(sdvo_cmd_names))
>> - DRM_LOG_KMS("(%02X)", cmd);
>> - DRM_LOG_KMS("\n");
>> + if (i == ARRAY_SIZE(sdvo_cmd_names)) {
>> + BUF_PRINT("(%02X)", cmd);
>> + }
>> + BUG_ON(pos >= BUF_LEN - 1);
>
> pos >= BUF_LEN is enough if you choose not to BUG_ON on each scnprintf
See above, with snprintf we need to make sure we have a terminating 0.
Without that the printk will go rampant.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
