Re: [PATCH] drm: actually remove the newline for CRC source name.
Thanks for the quick follow-up to Sam. Drop the word "actually" from the patch subject line. It's generally helpful to generate a 2nd version of the patch with '-v 2', and to leave a description what v2 changed. Also CC anyone who previously commented. On 2019-06-05 2:35 p.m., Dingchen Zhang wrote: > 'n-1' is the index to replace the newline character of CRC source name. > Add here: v2: Update patch subject (Sam) > Cc: Leo Li > Cc: Harry Wentland > Signed-off-by: Dingchen Zhang > --- > drivers/gpu/drm/drm_debugfs_crc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_debugfs_crc.c > b/drivers/gpu/drm/drm_debugfs_crc.c > index 585169f0dcc5..e20adef9d623 100644 > --- a/drivers/gpu/drm/drm_debugfs_crc.c > +++ b/drivers/gpu/drm/drm_debugfs_crc.c > @@ -131,8 +131,8 @@ static ssize_t crc_control_write(struct file *file, const > char __user *ubuf, > if (IS_ERR(source)) > return PTR_ERR(source); > > - if (source[len] == '\n') > - source[len] = '\0'; > + if (source[len-1] == '\n') > + source[len-1] = '\0'; > As Sam mentioned, you'll want this to be + if (source[len - 1] == '\n') + source[len - 1] = '\0'; I forgot to mention this to you before, but please run ./scripts/checkpatch.pl on your patches before sending them and fix up any errors or warnings. Harry > ret = crtc->funcs->verify_crc_source(crtc, source, _cnt); > if (ret) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: actually remove the newline for CRC source name.
Hi Dingchen. Thanks for the quick follow-up. On Wed, Jun 05, 2019 at 02:35:56PM -0400, Dingchen Zhang wrote: > 'n-1' is the index to replace the newline character of CRC source name. subject is much better now. It would be fine if the body of the changelog conveyed the same message. The body explains what the patch does, it is better to focus on why the patch does what is do. So maybe a short explanation that userspace may transfer a newine, and that this terminating newline is replaced by a '\0' to avoid followup isses. > > Cc: Leo Li > Cc: Harry Wentland Please add a space after the name, before the '<'. This is also suggested by checkpatch.pl. > Signed-off-by: Dingchen Zhang > --- > drivers/gpu/drm/drm_debugfs_crc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_debugfs_crc.c > b/drivers/gpu/drm/drm_debugfs_crc.c > index 585169f0dcc5..e20adef9d623 100644 > --- a/drivers/gpu/drm/drm_debugfs_crc.c > +++ b/drivers/gpu/drm/drm_debugfs_crc.c > @@ -131,8 +131,8 @@ static ssize_t crc_control_write(struct file *file, const > char __user *ubuf, > if (IS_ERR(source)) > return PTR_ERR(source); > > - if (source[len] == '\n') > - source[len] = '\0'; > + if (source[len-1] == '\n') > + source[len-1] = '\0'; You did not add spaces around operators as requested. Whith the above things fixed: Reviewed-by: Sam Ravnborg > > ret = crtc->funcs->verify_crc_source(crtc, source, _cnt); > if (ret) > -- > 2.17.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: actually remove the newline for CRC source name.
'n-1' is the index to replace the newline character of CRC source name. Cc: Leo Li Cc: Harry Wentland Signed-off-by: Dingchen Zhang --- drivers/gpu/drm/drm_debugfs_crc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c index 585169f0dcc5..e20adef9d623 100644 --- a/drivers/gpu/drm/drm_debugfs_crc.c +++ b/drivers/gpu/drm/drm_debugfs_crc.c @@ -131,8 +131,8 @@ static ssize_t crc_control_write(struct file *file, const char __user *ubuf, if (IS_ERR(source)) return PTR_ERR(source); - if (source[len] == '\n') - source[len] = '\0'; + if (source[len-1] == '\n') + source[len-1] = '\0'; ret = crtc->funcs->verify_crc_source(crtc, source, _cnt); if (ret) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
