[U-Boot] [PATCH] fs/fat: debug-print file read position during file_fat_read_at()

[Andreas Dannenberg]

Heinrich,

On Tue, Aug 14, 2018 at 07:40:14AM +0200, Heinrich Schuchardt wrote:
> On 08/14/2018 04:35 AM, Andreas Dannenberg wrote:
> > In order to make the debug print in file_fat_read_at() a tad more useful,
> > show the offset the file is being read at alongside the filename.
> > 
> > Suggested-by: Tero Kristo <t-kristo at ti.com>
> > Signed-off-by: Andreas Dannenberg <dannenberg at ti.com>
> > ---
> > 
> > Small addition but helpful nevertheless as none of the other debug prints
> > embedded into fat.c seems to output this info. Without that addition it
> > would just tell you the same file name a couple of times when reading
> > from an DTB/ITB file for example. With the offset being shown it's easier
> > to correlate/debug the loading process.
> > 
> > Oddly (and I haven't fully debugged this) but in order to really get _any_ of
> > the debug() prints working in fat.c in my particulat setup, in addition to
> > the usual '#define DEBUG' I also had to explicitly define _DEBUG to '1',
> > something that should already be taken care of by log.c...
> > 
> > 
> >  fs/fat/fat.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> > index 4efe8a3eda..4b722fc5ca 100644
> > --- a/fs/fat/fat.c
> > +++ b/fs/fat/fat.c
> > @@ -1095,7 +1095,7 @@ int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
> >  	if (ret)
> >  		goto out_free_both;
> >  
> > -	debug("reading %s\n", filename);
> > +	debug("reading %s at pos %llu\n", filename, pos);
> >  	ret = get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread);
> >  
> >  out_free_both:
> > 
> 
> This seems to duplicate
> [PATCH 1/1] fat: provide position in debug message
> https://lists.denx.de/pipermail/u-boot/2018-August/337850.html

Interesting, I had specifically searched for this to be potentially
there already, but I searched by prefix "fs/fat" which you used in your
earlier patch, however the prefix had changed in your later patch :)

> 
> The only difference is that you print a decimal number instead of a hex
> number.
> 
> Using %llu is consistent with
> fs/fat/fat.c:331:
> debug("Read position past EOF: %llu\n", pos);

Yes that's exactly why I used decimal. Also when using hex IMHO it would
have been better to prefix with 0x to minimize confusion, something that
got me into trouble in my early U-Boot days (it's often not clear what
is hex and what is dec, but things have since improved on that front).


--
Andreas Dannenberg
Texas Instruments Inc

> 
> So I will withdraw my patch.
> 
> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>