| Message ID | 20180410091435.11621-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 10 Apr 2018 11:14:35 +0200, Chris Wilson <chris@chris-wilson.co.uk> wrote: > %phn is not a valid specifier, Well, implementation allows that as alias for default separator ;) switch (fmt[1]) { case 'C': separator = ':'; break; case 'D': separator = '-'; break; case 'N': separator = 0; break; default: separator = ' '; break; } > so I presume %phN was meant for an > encoded hex string with no separator No, default ' ' separator is desired as it is more readable: [] writing 05 45 00 00 00 34 a2 0d 40 00 00 00 01 00 00 00 vs [] writing 054500000034a20d4000000001000000 > (and not that the hex string should > be followed by the letter 'n'!). 'n' was never added to the output, as it was consumed by fmt specifier ;) > > drivers/gpu/drm/i915/intel_guc_ct.c:616 ctb_read() warn: '%ph' cannot be > followed by 'n' > drivers/gpu/drm/i915/intel_guc_ct.c:616 ctb_read() warn: '%ph' cannot be > followed by 'n' > drivers/gpu/drm/i915/intel_guc_ct.c:616 ctb_read() warn: '%ph' cannot be > followed by 'n' > drivers/gpu/drm/i915/intel_guc_ct.c:669 ct_handle_response() warn: '%ph' > cannot be followed by 'n' > drivers/gpu/drm/i915/intel_guc_ct.c:679 ct_handle_response() warn: '%ph' > cannot be followed by 'n' > drivers/gpu/drm/i915/intel_guc_ct.c:693 ct_handle_response() warn: '%ph' > cannot be followed by 'n' > drivers/gpu/drm/i915/intel_guc_ct.c:707 ct_handle_response() warn: '%ph' > cannot be followed by 'n' > drivers/gpu/drm/i915/intel_guc_ct.c:727 ct_process_request() warn: '%ph' > cannot be followed by 'n' > drivers/gpu/drm/i915/intel_guc_ct.c:803 ct_handle_request() warn: '%ph' > cannot be followed by 'n' hmm, I can't see these warnings, how to get them? > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> with s/%phN/%ph Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Thanks, Michal
Quoting Michal Wajdeczko (2018-04-10 11:57:03) > On Tue, 10 Apr 2018 11:14:35 +0200, Chris Wilson > <chris@chris-wilson.co.uk> wrote: > > > %phn is not a valid specifier, > > Well, implementation allows that as alias for default separator ;) > > switch (fmt[1]) { > case 'C': > separator = ':'; > break; > case 'D': > separator = '-'; > break; > case 'N': > separator = 0; > break; > default: > separator = ' '; > break; > } > > > so I presume %phN was meant for an > > encoded hex string with no separator > > No, default ' ' separator is desired as it is more readable: > > [] writing 05 45 00 00 00 34 a2 0d 40 00 00 00 01 00 00 00 > vs > [] writing 054500000034a20d4000000001000000 > > > (and not that the hex string should > > be followed by the letter 'n'!). > > 'n' was never added to the output, as it was consumed by fmt specifier ;) > > > > > drivers/gpu/drm/i915/intel_guc_ct.c:616 ctb_read() warn: '%ph' cannot be > > followed by 'n' > > drivers/gpu/drm/i915/intel_guc_ct.c:616 ctb_read() warn: '%ph' cannot be > > followed by 'n' > > drivers/gpu/drm/i915/intel_guc_ct.c:616 ctb_read() warn: '%ph' cannot be > > followed by 'n' > > drivers/gpu/drm/i915/intel_guc_ct.c:669 ct_handle_response() warn: '%ph' > > cannot be followed by 'n' > > drivers/gpu/drm/i915/intel_guc_ct.c:679 ct_handle_response() warn: '%ph' > > cannot be followed by 'n' > > drivers/gpu/drm/i915/intel_guc_ct.c:693 ct_handle_response() warn: '%ph' > > cannot be followed by 'n' > > drivers/gpu/drm/i915/intel_guc_ct.c:707 ct_handle_response() warn: '%ph' > > cannot be followed by 'n' > > drivers/gpu/drm/i915/intel_guc_ct.c:727 ct_process_request() warn: '%ph' > > cannot be followed by 'n' > > drivers/gpu/drm/i915/intel_guc_ct.c:803 ct_handle_request() warn: '%ph' > > cannot be followed by 'n' > > hmm, I can't see these warnings, how to get them? The extended format specifiers are known to smatch. I shall keep prodding to his this included it the litany of checkpatch tools. It has a higher false positive than say gcc, but not so high as checkpatch ;) But when it finds a bug, it is invaluable. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > with s/%phN/%ph Ok, will respin. -Chris
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c index 990141d5f195..319abf5a4710 100644 --- a/drivers/gpu/drm/i915/intel_guc_ct.c +++ b/drivers/gpu/drm/i915/intel_guc_ct.c @@ -361,7 +361,7 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb, (want_response ? GUC_CT_MSG_SEND_STATUS : 0) | (action[0] << GUC_CT_MSG_ACTION_SHIFT); - CT_DEBUG_DRIVER("CT: writing %*phn %*phn %*phn\n", + CT_DEBUG_DRIVER("CT: writing %*phN %*phN %*phN\n", 4, &header, 4, &fence, 4 * (len - 1), &action[1]); @@ -613,7 +613,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data) /* message len with header */ len = ct_header_get_len(data[0]) + 1; if (unlikely(len > (u32)available)) { - DRM_ERROR("CT: incomplete message %*phn %*phn %*phn\n", + DRM_ERROR("CT: incomplete message %*phN %*phN %*phN\n", 4, data, 4 * (head + available - 1 > size ? size - head : available - 1), &cmds[head], @@ -626,7 +626,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data) data[i] = cmds[head]; head = (head + 1) % size; } - CT_DEBUG_DRIVER("CT: received %*phn\n", 4 * len, data); + CT_DEBUG_DRIVER("CT: received %*phN\n", 4 * len, data); desc->head = head * 4; return 0; @@ -666,7 +666,7 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg) /* Response payload shall at least include fence and status */ if (unlikely(len < 2)) { - DRM_ERROR("CT: corrupted response %*phn\n", 4 * msglen, msg); + DRM_ERROR("CT: corrupted response %*phN\n", 4 * msglen, msg); return -EPROTO; } @@ -676,7 +676,7 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg) /* Format of the status follows RESPONSE message */ if (unlikely(!INTEL_GUC_MSG_IS_RESPONSE(status))) { - DRM_ERROR("CT: corrupted response %*phn\n", 4 * msglen, msg); + DRM_ERROR("CT: corrupted response %*phN\n", 4 * msglen, msg); return -EPROTO; } @@ -690,7 +690,7 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg) continue; } if (unlikely(datalen > req->response_len)) { - DRM_ERROR("CT: response %u too long %*phn\n", + DRM_ERROR("CT: response %u too long %*phN\n", req->fence, 4 * msglen, msg); datalen = 0; } @@ -704,7 +704,7 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg) spin_unlock(&ct->lock); if (!found) - DRM_ERROR("CT: unsolicited response %*phn\n", 4 * msglen, msg); + DRM_ERROR("CT: unsolicited response %*phN\n", 4 * msglen, msg); return 0; } @@ -713,7 +713,7 @@ static void ct_process_request(struct intel_guc_ct *ct, { struct intel_guc *guc = ct_to_guc(ct); - CT_DEBUG_DRIVER("CT: request %x %*phn\n", action, 4 * len, payload); + CT_DEBUG_DRIVER("CT: request %x %*phN\n", action, 4 * len, payload); switch (action) { case INTEL_GUC_ACTION_DEFAULT: @@ -724,7 +724,7 @@ static void ct_process_request(struct intel_guc_ct *ct, default: fail_unexpected: - DRM_ERROR("CT: unexpected request %x %*phn\n", + DRM_ERROR("CT: unexpected request %x %*phN\n", action, 4 * len, payload); break; } @@ -800,7 +800,7 @@ static int ct_handle_request(struct intel_guc_ct *ct, const u32 *msg) request = kmalloc(sizeof(*request) + 4 * msglen, GFP_ATOMIC); if (unlikely(!request)) { - DRM_ERROR("CT: dropping request %*phn\n", 4 * msglen, msg); + DRM_ERROR("CT: dropping request %*phN\n", 4 * msglen, msg); return 0; /* XXX: -ENOMEM ? */ } memcpy(request->msg, msg, 4 * msglen);
%phn is not a valid specifier, so I presume %phN was meant for an encoded hex string with no separator (and not that the hex string should be followed by the letter 'n'!). drivers/gpu/drm/i915/intel_guc_ct.c:616 ctb_read() warn: '%ph' cannot be followed by 'n' drivers/gpu/drm/i915/intel_guc_ct.c:616 ctb_read() warn: '%ph' cannot be followed by 'n' drivers/gpu/drm/i915/intel_guc_ct.c:616 ctb_read() warn: '%ph' cannot be followed by 'n' drivers/gpu/drm/i915/intel_guc_ct.c:669 ct_handle_response() warn: '%ph' cannot be followed by 'n' drivers/gpu/drm/i915/intel_guc_ct.c:679 ct_handle_response() warn: '%ph' cannot be followed by 'n' drivers/gpu/drm/i915/intel_guc_ct.c:693 ct_handle_response() warn: '%ph' cannot be followed by 'n' drivers/gpu/drm/i915/intel_guc_ct.c:707 ct_handle_response() warn: '%ph' cannot be followed by 'n' drivers/gpu/drm/i915/intel_guc_ct.c:727 ct_process_request() warn: '%ph' cannot be followed by 'n' drivers/gpu/drm/i915/intel_guc_ct.c:803 ct_handle_request() warn: '%ph' cannot be followed by 'n' Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> --- drivers/gpu/drm/i915/intel_guc_ct.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)