Re: [PATCH v2 1/2] s390/kexec_file: print some more error messages

[Philipp Rudo <prudo@redhat.com>]

Hi Heiko,

On Tue, 7 Dec 2021 16:29:55 +0100
Heiko Carstens <hca@linux.ibm.com> wrote:

> Hi Philipp,
> 
> On Tue, Dec 07, 2021 at 01:57:48PM +0100, Philipp Rudo wrote:
> > Be kind and give some more information on what went wrong.
> > 
> > Signed-off-by: Philipp Rudo <prudo@redhat.com>  
> ...
> > @@ -304,15 +306,22 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> >  		sym = (void *)pi->ehdr + symtab->sh_offset;
> >  		sym += ELF64_R_SYM(relas[i].r_info);
> >  
> > -		if (sym->st_shndx == SHN_UNDEF)
> > +		if (sym->st_shndx == SHN_UNDEF) {
> > +			pr_err("Undefined symbol\n");
> >  			return -ENOEXEC;
> > +		}
> >  
> > -		if (sym->st_shndx == SHN_COMMON)
> > +		if (sym->st_shndx == SHN_COMMON) {
> > +			pr_err("symbol in common section\n");
> >  			return -ENOEXEC;
> > +		}
> >  
> >  		if (sym->st_shndx >= pi->ehdr->e_shnum &&
> > -		    sym->st_shndx != SHN_ABS)
> > +		    sym->st_shndx != SHN_ABS) {
> > +			pr_err("Invalid section %d for symbol\n",
> > +			       sym->st_shndx);
> >  			return -ENOEXEC;  
> 
> So, if you add the additional error messages here, then I'd really
> like to see also the name of the symbol which is causing
> problems. Just like it is done on x86.
> Sorry for nitpicking :)

I actually dropped the name on purpose. At least for my work flow
knowing which check failed is more important as that already allows
me to search for, e.g. all undefined symbols as each of them can cause
trouble. Which symbol exactly triggered the check isn't that important.
In addition, the code to get a symbol name is rather ugly. At least
when you compare it to its usefulness.

But when you insist...

Philipp

P.S. To avoid an other round for that patch. Do you also want the two
     pr_debugs?