On Sat, Mar 01, 2025 at 03:54:25PM -0600, Glenn Washburn wrote:
> On Thu, 27 Feb 2025 17:27:29 +0100 > Daniel Kiper <dki...@net-space.pl> wrote:
> > On Thu, Feb 27, 2025 at 12:28:31AM -0600, Glenn Washburn wrote:
> > > On Mon, 24 Feb 2025 18:14:59 +0100 Daniel Kiper <dki...@net-space.pl>
> > > wrote:
> > > > On Mon, Jan 06, 2025 at 01:02:43AM -0600, Glenn Washburn wrote:
> > > > > For arguments that are paths to files, print the full path of the
> > > > > file.
> > > > >
> > > > > Signed-off-by: Glenn Washburn <developm...@efficientek.com>
> > > > > ---
> > > > > grub-core/commands/ls.c | 8 +++++---
> > > > > 1 file changed, 5 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/grub-core/commands/ls.c b/grub-core/commands/ls.c
> > > > > index e33c16158d63..384d3e3cede8 100644
> > > > > --- a/grub-core/commands/ls.c
> > > > > +++ b/grub-core/commands/ls.c
> > > > > @@ -98,6 +98,7 @@ static int
> > > > > print_file (const char *filename, const struct grub_dirhook_info
> > > > > *info,
> > > > > void *data)
> > > > > {
> > > > > + char *pathname = NULL;
> > > > > struct grub_ls_list_files_ctx *ctx = data;
> > > > >
> > > > > if ((! ctx->all) && (filename[0] == '.'))
> > > > > @@ -117,7 +118,6 @@ print_file (const char *filename, const struct
> > > > > grub_dirhook_info *info,
> > > > > if (! info->dir)
> > > > > {
> > > > > grub_file_t file;
> > > > > - char *pathname;
> > > > >
> > > > > if (ctx->dirname[grub_strlen (ctx->dirname) - 1] == '/')
> > > > > pathname = grub_xasprintf ("%s%s", ctx->dirname, filename);
> > > > > @@ -143,7 +143,6 @@ print_file (const char *filename, const struct
> > > > > grub_dirhook_info *info,
> > > > > else
> > > > > grub_xputs ("????????????");
> > > > >
> > > > > - grub_free (pathname);
> > > > > grub_errno = GRUB_ERR_NONE;
> > > > > }
> > > > > else
> > > > > @@ -165,7 +164,10 @@ print_file (const char *filename, const struct
> > > > > grub_dirhook_info *info,
> > > > > datetime.day, datetime.hour,
> > > > > datetime.minute, datetime.second);
> > > > > }
> > > > > - grub_printf ("%s%s\n", filename, info->dir ? "/" : "");
> > > > > + grub_printf ("%s%s\n", (ctx->filename) ? pathname : filename,
> > > >
> > > > "(pathname != NULL) ? pathname : filename" would not be more
> > > > natural/correct?
> > >
> > > I think this is equivalent. Yes, this does seem more intuitive. Do you
> > > want me to update and send a v5?
>
> Actually, I'm wrong here. I should've run the filesystem tests instead
> of just trying to reason through the code. This is not equivalent and
> the way I had it originally was correct. The reason is that we do not
> want to print a full path for files that are listed from directory
> recursion. (pathname != NULL) is true when we are printing a listing of
> a file, but that listing could be because a full path to the file was
> given as an argument to ls or because ls was listing the contents of
> the files parent directory. In the former, ctx->filename is set and we
> want to print the full path. In the latter, ctx->filename is not set
> and we do not want to print the full path because the full path to the
> parent directory will have been printed earlier.
So, I think this grub_printf() call begs for a few lines of comment then.
Otherwise it is confusing...
> So ignore the v5, I'll send a v6 reverting this change.
OK...
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
