| Message ID | 20190205111118.26842-2-felipe.balbi@linux.intel.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 1381a5113caf764f090b912b478663275e7b999e |
| Headers | show |
| Series | [1/2] usb: dwc3: trace: pass trace buffer size to decoding functions | expand |
Hi Felipe, On Tue, Feb 05, 2019 at 01:11:18PM +0200, Felipe Balbi wrote: > Now that buffer size is always passed around, we don't need to rely on > strcat anymore. > > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> > --- > drivers/usb/dwc3/debug.h | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h > index e925a6b73005..6759a7efd8d5 100644 > --- a/drivers/usb/dwc3/debug.h > +++ b/drivers/usb/dwc3/debug.h > @@ -545,21 +545,25 @@ static inline const char *dwc3_ep_event_string(char *str, size_t size, > status & DEPEVT_STATUS_TRANSFER_ACTIVE ? > " (Active)" : " (Not Active)"); > > + len = strlen(str); > + I don't see 'len' used after this. Did you mean to s/ret/len/ in the snprintf() calls below? Alternatively couldn't you just use 'ret' as an accumulator each time snprintf() is called to keep count of the bytes written to the buffer instead of needing to call strlen() [O(n) each time] multiple times? Thanks, Jack > /* Control Endpoints */ > if (epnum <= 1) { > int phase = DEPEVT_STATUS_CONTROL_PHASE(event->status); > > switch (phase) { > case DEPEVT_STATUS_CONTROL_DATA: > - strcat(str, " [Data Phase]"); > + snprintf(str + ret, size - ret, > + " [Data Phase]"); > break; > case DEPEVT_STATUS_CONTROL_STATUS: > - strcat(str, " [Status Phase]"); > + snprintf(str + ret, size - ret, > + " [Status Phase]"); > } > } > break; > case DWC3_DEPEVT_RXTXFIFOEVT: > - strcat(str, "FIFO"); > + snprintf(str + ret, size - ret, "FIFO"); > break; > case DWC3_DEPEVT_STREAMEVT: > status = event->status; > @@ -571,13 +575,13 @@ static inline const char *dwc3_ep_event_string(char *str, size_t size, > break; > case DEPEVT_STREAMEVT_NOTFOUND: > default: > - strcat(str, " Stream Not Found"); > + snprintf(str + ret, size - ret, " Stream Not Found"); > break; > } > > break; > case DWC3_DEPEVT_EPCMDCMPLT: > - strcat(str, "Endpoint Command Complete"); > + snprintf(str + ret, size - ret, "Endpoint Command Complete"); > break; > default: > snprintf(str, size, "UNKNOWN");
Hi, Jack Pham <jackp@codeaurora.org> writes: > On Tue, Feb 05, 2019 at 01:11:18PM +0200, Felipe Balbi wrote: >> Now that buffer size is always passed around, we don't need to rely on >> strcat anymore. >> >> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> >> --- >> drivers/usb/dwc3/debug.h | 14 +++++++++----- >> 1 file changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h >> index e925a6b73005..6759a7efd8d5 100644 >> --- a/drivers/usb/dwc3/debug.h >> +++ b/drivers/usb/dwc3/debug.h >> @@ -545,21 +545,25 @@ static inline const char *dwc3_ep_event_string(char *str, size_t size, >> status & DEPEVT_STATUS_TRANSFER_ACTIVE ? >> " (Active)" : " (Not Active)"); >> >> + len = strlen(str); >> + > > I don't see 'len' used after this. Did you mean to s/ret/len/ in the > snprintf() calls below? > > Alternatively couldn't you just use 'ret' as an accumulator each time > snprintf() is called to keep count of the bytes written to the buffer > instead of needing to call strlen() [O(n) each time] multiple times? yeah, this is leftover. Just forgot to remove it.
Felipe Balbi <felipe.balbi@linux.intel.com> writes: > Hi, > > Jack Pham <jackp@codeaurora.org> writes: >> On Tue, Feb 05, 2019 at 01:11:18PM +0200, Felipe Balbi wrote: >>> Now that buffer size is always passed around, we don't need to rely on >>> strcat anymore. >>> >>> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> >>> --- >>> drivers/usb/dwc3/debug.h | 14 +++++++++----- >>> 1 file changed, 9 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h >>> index e925a6b73005..6759a7efd8d5 100644 >>> --- a/drivers/usb/dwc3/debug.h >>> +++ b/drivers/usb/dwc3/debug.h >>> @@ -545,21 +545,25 @@ static inline const char *dwc3_ep_event_string(char *str, size_t size, >>> status & DEPEVT_STATUS_TRANSFER_ACTIVE ? >>> " (Active)" : " (Not Active)"); >>> >>> + len = strlen(str); >>> + >> >> I don't see 'len' used after this. Did you mean to s/ret/len/ in the >> snprintf() calls below? >> >> Alternatively couldn't you just use 'ret' as an accumulator each time >> snprintf() is called to keep count of the bytes written to the buffer >> instead of needing to call strlen() [O(n) each time] multiple times? > > yeah, this is leftover. Just forgot to remove it. Just to be clear, removed on a separate patch :-)
diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h index e925a6b73005..6759a7efd8d5 100644 --- a/drivers/usb/dwc3/debug.h +++ b/drivers/usb/dwc3/debug.h @@ -545,21 +545,25 @@ static inline const char *dwc3_ep_event_string(char *str, size_t size, status & DEPEVT_STATUS_TRANSFER_ACTIVE ? " (Active)" : " (Not Active)"); + len = strlen(str); + /* Control Endpoints */ if (epnum <= 1) { int phase = DEPEVT_STATUS_CONTROL_PHASE(event->status); switch (phase) { case DEPEVT_STATUS_CONTROL_DATA: - strcat(str, " [Data Phase]"); + snprintf(str + ret, size - ret, + " [Data Phase]"); break; case DEPEVT_STATUS_CONTROL_STATUS: - strcat(str, " [Status Phase]"); + snprintf(str + ret, size - ret, + " [Status Phase]"); } } break; case DWC3_DEPEVT_RXTXFIFOEVT: - strcat(str, "FIFO"); + snprintf(str + ret, size - ret, "FIFO"); break; case DWC3_DEPEVT_STREAMEVT: status = event->status; @@ -571,13 +575,13 @@ static inline const char *dwc3_ep_event_string(char *str, size_t size, break; case DEPEVT_STREAMEVT_NOTFOUND: default: - strcat(str, " Stream Not Found"); + snprintf(str + ret, size - ret, " Stream Not Found"); break; } break; case DWC3_DEPEVT_EPCMDCMPLT: - strcat(str, "Endpoint Command Complete"); + snprintf(str + ret, size - ret, "Endpoint Command Complete"); break; default: snprintf(str, size, "UNKNOWN");
Now that buffer size is always passed around, we don't need to rely on strcat anymore. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> --- drivers/usb/dwc3/debug.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)