Print errors before exit. Do not exit silently.

Signed-off-by: Changbin Du <changbin.du@huawei.com>
---
 hw/arm/boot.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index d480a7da02cf..17c01f299831 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -839,6 +839,7 @@ static ssize_t arm_load_elf(struct arm_boot_info *info, uint64_t *pentry,
                       1, data_swab, as);
     if (ret <= 0) {
         /* The header loaded but the image didn't */
+        error_report("could not load elf '%s'", info->kernel_filename);
         exit(1);
     }
 
@@ -1322,6 +1323,7 @@ void arm_load_kernel(ARMCPU *cpu, MachineState *ms, struct arm_boot_info *info)
      */
     if (!info->skip_dtb_autoload && have_dtb(info)) {
         if (arm_load_dtb(info->dtb_start, info, info->dtb_limit, as, ms) < 0) {
+            error_report("could not load dtb '%s'", info->dtb_filename);
             exit(1);
         }
     }
-- 
2.34.1

On Fri, 30 Aug 2024 at 08:24, Changbin Du <changbin.du@huawei.com> wrote:
>
> Print errors before exit. Do not exit silently.
>
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> ---
>  hw/arm/boot.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index d480a7da02cf..17c01f299831 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -839,6 +839,7 @@ static ssize_t arm_load_elf(struct arm_boot_info *info, uint64_t *pentry,
>                        1, data_swab, as);
>      if (ret <= 0) {
>          /* The header loaded but the image didn't */
> +        error_report("could not load elf '%s'", info->kernel_filename);
>          exit(1);

I agree we should print a message here.

>      }
>
> @@ -1322,6 +1323,7 @@ void arm_load_kernel(ARMCPU *cpu, MachineState *ms, struct arm_boot_info *info)
>       */
>      if (!info->skip_dtb_autoload && have_dtb(info)) {
>          if (arm_load_dtb(info->dtb_start, info, info->dtb_limit, as, ms) < 0) {
> +            error_report("could not load dtb '%s'", info->dtb_filename);
>              exit(1);

But for this case, arm_load_dtb() should be printing the error
message itself (which lets it be more precise about what went
wrong). Is there a code path where it fails to print an error?

thanks
-- PMM

On Fri, Aug 30, 2024 at 11:05:59AM +0100, Peter Maydell wrote:
> On Fri, 30 Aug 2024 at 08:24, Changbin Du <changbin.du@huawei.com> wrote:
> >
> > Print errors before exit. Do not exit silently.
> >
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > ---
> >  hw/arm/boot.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> > index d480a7da02cf..17c01f299831 100644
> > --- a/hw/arm/boot.c
> > +++ b/hw/arm/boot.c
> > @@ -839,6 +839,7 @@ static ssize_t arm_load_elf(struct arm_boot_info *info, uint64_t *pentry,
> >                        1, data_swab, as);
> >      if (ret <= 0) {
> >          /* The header loaded but the image didn't */
> > +        error_report("could not load elf '%s'", info->kernel_filename);
> >          exit(1);
> 
> I agree we should print a message here.
> 
> >      }
> >
> > @@ -1322,6 +1323,7 @@ void arm_load_kernel(ARMCPU *cpu, MachineState *ms, struct arm_boot_info *info)
> >       */
> >      if (!info->skip_dtb_autoload && have_dtb(info)) {
> >          if (arm_load_dtb(info->dtb_start, info, info->dtb_limit, as, ms) < 0) {
> > +            error_report("could not load dtb '%s'", info->dtb_filename);
> >              exit(1);
> 
> But for this case, arm_load_dtb() should be printing the error
> message itself (which lets it be more precise about what went
> wrong). Is there a code path where it fails to print an error?
>
Seems not. arm_load_dtb() prints errors on all of its failure paths. So this
change should be removed.

> thanks
> -- PMM

-- 
Cheers,
Changbin Du