On Thu, Aug 01, 2024 at 08:00:21AM -0600, Cavitt, Jonathan wrote: > -----Original Message----- > From: Brost, Matthew <matthew.brost@xxxxxxxxx> > Sent: Wednesday, July 31, 2024 5:03 PM > To: Cavitt, Jonathan <jonathan.cavitt@xxxxxxxxx> > Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; maarten.lankhorst@xxxxxxxxxxxxxxx; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx> > 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@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Matthew Brost > > > Sent: Wednesday, July 31, 2024 2:32 PM > > > To: intel-xe@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx > > > Cc: maarten.lankhorst@xxxxxxxxxxxxxxx; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx> > > > 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@xxxxxxxxxxxxxxx> > > > > Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx> > > > > > > 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@xxxxxxxxx> > > > -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 > > > > > > > > > >