diff mbox series

[v4,2/3] drm/printer: Allow NULL data in devcoredump printer

Message ID 20240731213221.2523989-3-matthew.brost@intel.com (mailing list archive)
State New, archived
Headers show
Series Faster devcoredump and fixes | expand

Commit Message

Matthew Brost July 31, 2024, 9:32 p.m. UTC
Useful to determine size of devcoreump before writing it out.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/drm_print.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Cavitt, Jonathan July 31, 2024, 10:22 p.m. UTC | #1
-----Original Message-----
From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Matthew Brost
Sent: Wednesday, July 31, 2024 2:32 PM
To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Cc: maarten.lankhorst@linux.intel.com; Vivi, Rodrigo <rodrigo.vivi@intel.com>
Subject: [PATCH v4 2/3] drm/printer: Allow NULL data in devcoredump printer
> 
> Useful to determine size of devcoreump before writing it out.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>

It seems this patch prevents us from copying strings into the data field if the data
field hasn't been initialized.  I'm not certain if it could ever be uninitialized at this
point, but I recognize it as good practice to check just in case regardless.

Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
-Jonathan Cavitt

> ---
>  drivers/gpu/drm/drm_print.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index cf24dfdeb6b2..a1a4de9f9c44 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -100,8 +100,9 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
>  			copy = iterator->remain;
>  
>  		/* Copy out the bit of the string that we need */
> -		memcpy(iterator->data,
> -			str + (iterator->start - iterator->offset), copy);
> +		if (iterator->data)
> +			memcpy(iterator->data,
> +				str + (iterator->start - iterator->offset), copy);
>  
>  		iterator->offset = iterator->start + copy;
>  		iterator->remain -= copy;
> @@ -110,7 +111,8 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
>  
>  		len = min_t(ssize_t, strlen(str), iterator->remain);
>  
> -		memcpy(iterator->data + pos, str, len);
> +		if (iterator->data)
> +			memcpy(iterator->data + pos, str, len);
>  
>  		iterator->offset += len;
>  		iterator->remain -= len;
> @@ -140,8 +142,9 @@ void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
>  	if ((iterator->offset >= iterator->start) && (len < iterator->remain)) {
>  		ssize_t pos = iterator->offset - iterator->start;
>  
> -		snprintf(((char *) iterator->data) + pos,
> -			iterator->remain, "%pV", vaf);
> +		if (iterator->data)
> +			snprintf(((char *) iterator->data) + pos,
> +				iterator->remain, "%pV", vaf);
>  
>  		iterator->offset += len;
>  		iterator->remain -= len;
> -- 
> 2.34.1
> 
>
Matthew Brost Aug. 1, 2024, 12:02 a.m. UTC | #2
On Wed, Jul 31, 2024 at 04:22:03PM -0600, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Matthew Brost
> Sent: Wednesday, July 31, 2024 2:32 PM
> To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: maarten.lankhorst@linux.intel.com; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Subject: [PATCH v4 2/3] drm/printer: Allow NULL data in devcoredump printer
> > 
> > Useful to determine size of devcoreump before writing it out.
> > 
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> 
> It seems this patch prevents us from copying strings into the data field if the data
> field hasn't been initialized.  I'm not certain if it could ever be uninitialized at this
> point, but I recognize it as good practice to check just in case regardless.
> 

That's not the intent. The intent to call the print functions with NULL
data so the printer can calculate the size of buffer required to print
out all the devcoredump data.

Matt

> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> -Jonathan Cavitt
> 
> > ---
> >  drivers/gpu/drm/drm_print.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> > index cf24dfdeb6b2..a1a4de9f9c44 100644
> > --- a/drivers/gpu/drm/drm_print.c
> > +++ b/drivers/gpu/drm/drm_print.c
> > @@ -100,8 +100,9 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
> >  			copy = iterator->remain;
> >  
> >  		/* Copy out the bit of the string that we need */
> > -		memcpy(iterator->data,
> > -			str + (iterator->start - iterator->offset), copy);
> > +		if (iterator->data)
> > +			memcpy(iterator->data,
> > +				str + (iterator->start - iterator->offset), copy);
> >  
> >  		iterator->offset = iterator->start + copy;
> >  		iterator->remain -= copy;
> > @@ -110,7 +111,8 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
> >  
> >  		len = min_t(ssize_t, strlen(str), iterator->remain);
> >  
> > -		memcpy(iterator->data + pos, str, len);
> > +		if (iterator->data)
> > +			memcpy(iterator->data + pos, str, len);
> >  
> >  		iterator->offset += len;
> >  		iterator->remain -= len;
> > @@ -140,8 +142,9 @@ void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
> >  	if ((iterator->offset >= iterator->start) && (len < iterator->remain)) {
> >  		ssize_t pos = iterator->offset - iterator->start;
> >  
> > -		snprintf(((char *) iterator->data) + pos,
> > -			iterator->remain, "%pV", vaf);
> > +		if (iterator->data)
> > +			snprintf(((char *) iterator->data) + pos,
> > +				iterator->remain, "%pV", vaf);
> >  
> >  		iterator->offset += len;
> >  		iterator->remain -= len;
> > -- 
> > 2.34.1
> > 
> >
Jani Nikula Aug. 1, 2024, 8:05 a.m. UTC | #3
On Wed, 31 Jul 2024, Matthew Brost <matthew.brost@intel.com> wrote:
> Useful to determine size of devcoreump before writing it out.

I find it useful to have this special case documented, with an example,
so it's easier to see how handy this really is.

BR,
Jani.


>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/drm_print.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index cf24dfdeb6b2..a1a4de9f9c44 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -100,8 +100,9 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
>  			copy = iterator->remain;
>  
>  		/* Copy out the bit of the string that we need */
> -		memcpy(iterator->data,
> -			str + (iterator->start - iterator->offset), copy);
> +		if (iterator->data)
> +			memcpy(iterator->data,
> +				str + (iterator->start - iterator->offset), copy);
>  
>  		iterator->offset = iterator->start + copy;
>  		iterator->remain -= copy;
> @@ -110,7 +111,8 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
>  
>  		len = min_t(ssize_t, strlen(str), iterator->remain);
>  
> -		memcpy(iterator->data + pos, str, len);
> +		if (iterator->data)
> +			memcpy(iterator->data + pos, str, len);
>  
>  		iterator->offset += len;
>  		iterator->remain -= len;
> @@ -140,8 +142,9 @@ void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
>  	if ((iterator->offset >= iterator->start) && (len < iterator->remain)) {
>  		ssize_t pos = iterator->offset - iterator->start;
>  
> -		snprintf(((char *) iterator->data) + pos,
> -			iterator->remain, "%pV", vaf);
> +		if (iterator->data)
> +			snprintf(((char *) iterator->data) + pos,
> +				iterator->remain, "%pV", vaf);
>  
>  		iterator->offset += len;
>  		iterator->remain -= len;
Cavitt, Jonathan Aug. 1, 2024, 2 p.m. UTC | #4
-----Original Message-----
From: Brost, Matthew <matthew.brost@intel.com> 
Sent: Wednesday, July 31, 2024 5:03 PM
To: Cavitt, Jonathan <jonathan.cavitt@intel.com>
Cc: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org; maarten.lankhorst@linux.intel.com; Vivi, Rodrigo <rodrigo.vivi@intel.com>
Subject: Re: [PATCH v4 2/3] drm/printer: Allow NULL data in devcoredump printer
> 
> On Wed, Jul 31, 2024 at 04:22:03PM -0600, Cavitt, Jonathan wrote:
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Matthew Brost
> > Sent: Wednesday, July 31, 2024 2:32 PM
> > To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> > Cc: maarten.lankhorst@linux.intel.com; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> > Subject: [PATCH v4 2/3] drm/printer: Allow NULL data in devcoredump printer
> > > 
> > > Useful to determine size of devcoreump before writing it out.
> > > 
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > 
> > It seems this patch prevents us from copying strings into the data field if the data
> > field hasn't been initialized.  I'm not certain if it could ever be uninitialized at this
> > point, but I recognize it as good practice to check just in case regardless.
> > 
> 
> That's not the intent. The intent to call the print functions with NULL
> data so the printer can calculate the size of buffer required to print
> out all the devcoredump data.

So if iterator->data is NULL, you want to NOT copy into it?
-Jonathan Cavitt

> 
> Matt
> 
> > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > -Jonathan Cavitt
> > 
> > > ---
> > >  drivers/gpu/drm/drm_print.c | 13 ++++++++-----
> > >  1 file changed, 8 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> > > index cf24dfdeb6b2..a1a4de9f9c44 100644
> > > --- a/drivers/gpu/drm/drm_print.c
> > > +++ b/drivers/gpu/drm/drm_print.c
> > > @@ -100,8 +100,9 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
> > >  			copy = iterator->remain;
> > >  
> > >  		/* Copy out the bit of the string that we need */
> > > -		memcpy(iterator->data,
> > > -			str + (iterator->start - iterator->offset), copy);
> > > +		if (iterator->data)
> > > +			memcpy(iterator->data,
> > > +				str + (iterator->start - iterator->offset), copy);
> > >  
> > >  		iterator->offset = iterator->start + copy;
> > >  		iterator->remain -= copy;
> > > @@ -110,7 +111,8 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
> > >  
> > >  		len = min_t(ssize_t, strlen(str), iterator->remain);
> > >  
> > > -		memcpy(iterator->data + pos, str, len);
> > > +		if (iterator->data)
> > > +			memcpy(iterator->data + pos, str, len);
> > >  
> > >  		iterator->offset += len;
> > >  		iterator->remain -= len;
> > > @@ -140,8 +142,9 @@ void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
> > >  	if ((iterator->offset >= iterator->start) && (len < iterator->remain)) {
> > >  		ssize_t pos = iterator->offset - iterator->start;
> > >  
> > > -		snprintf(((char *) iterator->data) + pos,
> > > -			iterator->remain, "%pV", vaf);
> > > +		if (iterator->data)
> > > +			snprintf(((char *) iterator->data) + pos,
> > > +				iterator->remain, "%pV", vaf);
> > >  
> > >  		iterator->offset += len;
> > >  		iterator->remain -= len;
> > > -- 
> > > 2.34.1
> > > 
> > > 
>
Matthew Brost Aug. 1, 2024, 2:27 p.m. UTC | #5
On Thu, Aug 01, 2024 at 11:05:21AM +0300, Jani Nikula wrote:
> On Wed, 31 Jul 2024, Matthew Brost <matthew.brost@intel.com> wrote:
> > Useful to determine size of devcoreump before writing it out.
> 
> I find it useful to have this special case documented, with an example,
> so it's easier to see how handy this really is.
> 

Good idea, will add some kernel doc explaining the problem devcoredump
in Xe and how use a devcoredump printer with NULL to make it faster.

Matt

> BR,
> Jani.
> 
> 
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> >  drivers/gpu/drm/drm_print.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> > index cf24dfdeb6b2..a1a4de9f9c44 100644
> > --- a/drivers/gpu/drm/drm_print.c
> > +++ b/drivers/gpu/drm/drm_print.c
> > @@ -100,8 +100,9 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
> >  			copy = iterator->remain;
> >  
> >  		/* Copy out the bit of the string that we need */
> > -		memcpy(iterator->data,
> > -			str + (iterator->start - iterator->offset), copy);
> > +		if (iterator->data)
> > +			memcpy(iterator->data,
> > +				str + (iterator->start - iterator->offset), copy);
> >  
> >  		iterator->offset = iterator->start + copy;
> >  		iterator->remain -= copy;
> > @@ -110,7 +111,8 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
> >  
> >  		len = min_t(ssize_t, strlen(str), iterator->remain);
> >  
> > -		memcpy(iterator->data + pos, str, len);
> > +		if (iterator->data)
> > +			memcpy(iterator->data + pos, str, len);
> >  
> >  		iterator->offset += len;
> >  		iterator->remain -= len;
> > @@ -140,8 +142,9 @@ void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
> >  	if ((iterator->offset >= iterator->start) && (len < iterator->remain)) {
> >  		ssize_t pos = iterator->offset - iterator->start;
> >  
> > -		snprintf(((char *) iterator->data) + pos,
> > -			iterator->remain, "%pV", vaf);
> > +		if (iterator->data)
> > +			snprintf(((char *) iterator->data) + pos,
> > +				iterator->remain, "%pV", vaf);
> >  
> >  		iterator->offset += len;
> >  		iterator->remain -= len;
> 
> -- 
> Jani Nikula, Intel
Matthew Brost Aug. 1, 2024, 2:29 p.m. UTC | #6
On Thu, Aug 01, 2024 at 08:00:21AM -0600, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Brost, Matthew <matthew.brost@intel.com> 
> Sent: Wednesday, July 31, 2024 5:03 PM
> To: Cavitt, Jonathan <jonathan.cavitt@intel.com>
> Cc: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org; maarten.lankhorst@linux.intel.com; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Subject: Re: [PATCH v4 2/3] drm/printer: Allow NULL data in devcoredump printer
> > 
> > On Wed, Jul 31, 2024 at 04:22:03PM -0600, Cavitt, Jonathan wrote:
> > > -----Original Message-----
> > > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Matthew Brost
> > > Sent: Wednesday, July 31, 2024 2:32 PM
> > > To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> > > Cc: maarten.lankhorst@linux.intel.com; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> > > Subject: [PATCH v4 2/3] drm/printer: Allow NULL data in devcoredump printer
> > > > 
> > > > Useful to determine size of devcoreump before writing it out.
> > > > 
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > > 
> > > It seems this patch prevents us from copying strings into the data field if the data
> > > field hasn't been initialized.  I'm not certain if it could ever be uninitialized at this
> > > point, but I recognize it as good practice to check just in case regardless.
> > > 
> > 
> > That's not the intent. The intent to call the print functions with NULL
> > data so the printer can calculate the size of buffer required to print
> > out all the devcoredump data.
> 
> So if iterator->data is NULL, you want to NOT copy into it?
> -Jonathan Cavitt

Yes, exactly. Run devcoredump printer with NULL data to get size, alloc
data based on devcoredump printer offset, run it again with a valid data
pointer to print.

Matt

> 
> > 
> > Matt
> > 
> > > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > > -Jonathan Cavitt
> > > 
> > > > ---
> > > >  drivers/gpu/drm/drm_print.c | 13 ++++++++-----
> > > >  1 file changed, 8 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> > > > index cf24dfdeb6b2..a1a4de9f9c44 100644
> > > > --- a/drivers/gpu/drm/drm_print.c
> > > > +++ b/drivers/gpu/drm/drm_print.c
> > > > @@ -100,8 +100,9 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
> > > >  			copy = iterator->remain;
> > > >  
> > > >  		/* Copy out the bit of the string that we need */
> > > > -		memcpy(iterator->data,
> > > > -			str + (iterator->start - iterator->offset), copy);
> > > > +		if (iterator->data)
> > > > +			memcpy(iterator->data,
> > > > +				str + (iterator->start - iterator->offset), copy);
> > > >  
> > > >  		iterator->offset = iterator->start + copy;
> > > >  		iterator->remain -= copy;
> > > > @@ -110,7 +111,8 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
> > > >  
> > > >  		len = min_t(ssize_t, strlen(str), iterator->remain);
> > > >  
> > > > -		memcpy(iterator->data + pos, str, len);
> > > > +		if (iterator->data)
> > > > +			memcpy(iterator->data + pos, str, len);
> > > >  
> > > >  		iterator->offset += len;
> > > >  		iterator->remain -= len;
> > > > @@ -140,8 +142,9 @@ void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
> > > >  	if ((iterator->offset >= iterator->start) && (len < iterator->remain)) {
> > > >  		ssize_t pos = iterator->offset - iterator->start;
> > > >  
> > > > -		snprintf(((char *) iterator->data) + pos,
> > > > -			iterator->remain, "%pV", vaf);
> > > > +		if (iterator->data)
> > > > +			snprintf(((char *) iterator->data) + pos,
> > > > +				iterator->remain, "%pV", vaf);
> > > >  
> > > >  		iterator->offset += len;
> > > >  		iterator->remain -= len;
> > > > -- 
> > > > 2.34.1
> > > > 
> > > > 
> >
Cavitt, Jonathan Aug. 1, 2024, 2:38 p.m. UTC | #7
-----Original Message-----
From: Brost, Matthew <matthew.brost@intel.com> 
Sent: Thursday, August 1, 2024 7:30 AM
To: Cavitt, Jonathan <jonathan.cavitt@intel.com>
Cc: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org; maarten.lankhorst@linux.intel.com; Vivi, Rodrigo <rodrigo.vivi@intel.com>
Subject: Re: [PATCH v4 2/3] drm/printer: Allow NULL data in devcoredump printer
> 
> On Thu, Aug 01, 2024 at 08:00:21AM -0600, Cavitt, Jonathan wrote:
> > -----Original Message-----
> > From: Brost, Matthew <matthew.brost@intel.com> 
> > Sent: Wednesday, July 31, 2024 5:03 PM
> > To: Cavitt, Jonathan <jonathan.cavitt@intel.com>
> > Cc: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org; maarten.lankhorst@linux.intel.com; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> > Subject: Re: [PATCH v4 2/3] drm/printer: Allow NULL data in devcoredump printer
> > > 
> > > On Wed, Jul 31, 2024 at 04:22:03PM -0600, Cavitt, Jonathan wrote:
> > > > -----Original Message-----
> > > > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Matthew Brost
> > > > Sent: Wednesday, July 31, 2024 2:32 PM
> > > > To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> > > > Cc: maarten.lankhorst@linux.intel.com; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> > > > Subject: [PATCH v4 2/3] drm/printer: Allow NULL data in devcoredump printer
> > > > > 
> > > > > Useful to determine size of devcoreump before writing it out.
> > > > > 
> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > > > 
> > > > It seems this patch prevents us from copying strings into the data field if the data
> > > > field hasn't been initialized.  I'm not certain if it could ever be uninitialized at this
> > > > point, but I recognize it as good practice to check just in case regardless.
> > > > 
> > > 
> > > That's not the intent. The intent to call the print functions with NULL
> > > data so the printer can calculate the size of buffer required to print
> > > out all the devcoredump data.
> > 
> > So if iterator->data is NULL, you want to NOT copy into it?
> > -Jonathan Cavitt
> 
> Yes, exactly. Run devcoredump printer with NULL data to get size, alloc
> data based on devcoredump printer offset, run it again with a valid data
> pointer to print.

Okay, I think I understand.  That might be good to add to the commit message,
then.  Something like:

"""
We want to determine the size of the devcoredump before writing it out.
To that end, we will run the devcoredump printer with NULL data to get
the size, alloc data based on the generated offset, then run the
devcorecump again with a valid data pointer to print.  This necessitates
not writing data to the data pointer on the initial pass, when it is NULL.
"""

Maybe that's a bit overboard?  In either case, my RB still stands,
regardless of if the commit message is updated or not.
-Jonathan Cavitt

> 
> Matt
> 
> > 
> > > 
> > > Matt
> > > 
> > > > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > > > -Jonathan Cavitt
> > > > 
> > > > > ---
> > > > >  drivers/gpu/drm/drm_print.c | 13 ++++++++-----
> > > > >  1 file changed, 8 insertions(+), 5 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> > > > > index cf24dfdeb6b2..a1a4de9f9c44 100644
> > > > > --- a/drivers/gpu/drm/drm_print.c
> > > > > +++ b/drivers/gpu/drm/drm_print.c
> > > > > @@ -100,8 +100,9 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
> > > > >  			copy = iterator->remain;
> > > > >  
> > > > >  		/* Copy out the bit of the string that we need */
> > > > > -		memcpy(iterator->data,
> > > > > -			str + (iterator->start - iterator->offset), copy);
> > > > > +		if (iterator->data)
> > > > > +			memcpy(iterator->data,
> > > > > +				str + (iterator->start - iterator->offset), copy);
> > > > >  
> > > > >  		iterator->offset = iterator->start + copy;
> > > > >  		iterator->remain -= copy;
> > > > > @@ -110,7 +111,8 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str)
> > > > >  
> > > > >  		len = min_t(ssize_t, strlen(str), iterator->remain);
> > > > >  
> > > > > -		memcpy(iterator->data + pos, str, len);
> > > > > +		if (iterator->data)
> > > > > +			memcpy(iterator->data + pos, str, len);
> > > > >  
> > > > >  		iterator->offset += len;
> > > > >  		iterator->remain -= len;
> > > > > @@ -140,8 +142,9 @@ void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
> > > > >  	if ((iterator->offset >= iterator->start) && (len < iterator->remain)) {
> > > > >  		ssize_t pos = iterator->offset - iterator->start;
> > > > >  
> > > > > -		snprintf(((char *) iterator->data) + pos,
> > > > > -			iterator->remain, "%pV", vaf);
> > > > > +		if (iterator->data)
> > > > > +			snprintf(((char *) iterator->data) + pos,
> > > > > +				iterator->remain, "%pV", vaf);
> > > > >  
> > > > >  		iterator->offset += len;
> > > > >  		iterator->remain -= len;
> > > > > -- 
> > > > > 2.34.1
> > > > > 
> > > > > 
> > > 
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index cf24dfdeb6b2..a1a4de9f9c44 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -100,8 +100,9 @@  void __drm_puts_coredump(struct drm_printer *p, const char *str)
 			copy = iterator->remain;
 
 		/* Copy out the bit of the string that we need */
-		memcpy(iterator->data,
-			str + (iterator->start - iterator->offset), copy);
+		if (iterator->data)
+			memcpy(iterator->data,
+				str + (iterator->start - iterator->offset), copy);
 
 		iterator->offset = iterator->start + copy;
 		iterator->remain -= copy;
@@ -110,7 +111,8 @@  void __drm_puts_coredump(struct drm_printer *p, const char *str)
 
 		len = min_t(ssize_t, strlen(str), iterator->remain);
 
-		memcpy(iterator->data + pos, str, len);
+		if (iterator->data)
+			memcpy(iterator->data + pos, str, len);
 
 		iterator->offset += len;
 		iterator->remain -= len;
@@ -140,8 +142,9 @@  void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
 	if ((iterator->offset >= iterator->start) && (len < iterator->remain)) {
 		ssize_t pos = iterator->offset - iterator->start;
 
-		snprintf(((char *) iterator->data) + pos,
-			iterator->remain, "%pV", vaf);
+		if (iterator->data)
+			snprintf(((char *) iterator->data) + pos,
+				iterator->remain, "%pV", vaf);
 
 		iterator->offset += len;
 		iterator->remain -= len;