Re: [PATCH] powerpc: Fix typos and field name in comments

[Gustavo Romero <gromero at linux dot vnet dot ibm dot com>]

Hi Tulio,

On 08/13/2019 03:28 PM, Tulio Magno Quites Machado Filho wrote:
> Hi Gustavo,
>
> The patch looks mostly good to me except for one line where I have a question.
> I plan to push it after getting a confirmation from you.
>
> Gustavo Romero <gromero@linux.vnet.ibm.com> writes:
>
> > Fix a couple of typos and v_regs field name in mcontext_t.
>
>
> > diff --git a/ChangeLog b/ChangeLog
> > index 225da01..39f50b8 100644
> > --- a/ChangeLog
> > +++ b/ChangeLog
> > @@ -1,3 +1,8 @@
> > +2019-08-12  Gustavo Romero  <gromero@linux.vnet.ibm.com>
> > +
> > +	* sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h: Fix typos and
> > +	field name in mcontext_t struct.
> > +
>
> Remember to put the ChangeLog in the commit message for you next patch.  ;-)
> For more information:
> https://sourceware.org/glibc/wiki/Contribution%20checklist#Properly_Formatted_GNU_ChangeLog

Got it.


> > diff --git a/sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h b/sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h
> > index 640381a..6093ade 100644
> > --- a/sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h
> > +++ b/sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h
> > @@ -75,12 +75,12 @@ typedef struct
> >    * a sigcontext.  For older kernel (without Altivec) the sigcontext matches
> >    * the mcontext upto but not including the v_regs field.  For kernels that
> >    * don't set AT_HWCAP or return AT_HWCAP without PPC_FEATURE_HAS_ALTIVEC the
> > - * v_regs field may not exist and should not be referenced.  The v_regd field
> > - * can be refernced safely only after verifying that PPC_FEATURE_HAS_ALTIVEC
> > + * v_regs field may not exist and should not be referenced.  The v_regs field
> > + * can be referenced safely only after verifying that PPC_FEATURE_HAS_ALTIVEC
>
> OK.
>
> >   
> >   /* Number of general registers.  */
> > -# define __NGREG	48	/* includes r0-r31, nip, msr, lr, etc.   */
> > +# define __NGREG	48	/* includes r0-r31, nip, msr, lr, etc.  */
>
> I'm afraid this modification should not be here.
> Or am I missing a fix here?

I see 3 spaces in the original version. Since in that file I found comments
ending with 1 or 2 spaces before "*/" I thought it would be good to remove
at least 1 space from these 3 spaces. But we can discard that nit if you deem it
not right, sure.


> > @@ -136,7 +136,7 @@ typedef struct {
> >    * either NULL (if this processor does not support the VMX feature) or the
> >    * address of the first quadword within the allocated (vmx_reserve) area.
> >    *
> > - * The pointer (v_regs) of vector type (elf_vrreg_t) is essentually
> > + * The pointer (v_regs) of vector type (elf_vrreg_t) is essentially
>
> OK.
>
> Thanks,

Thanks for reviewing and sponsoring it.


Best regards,
Gustavo