[ARM] Fix _unlink prototype under ARM_RDI_MONITOR.

Message ID CAKdteOamD3eshDRnMwySYzXi8X_iLQLuCk+ovTD3UcBHcAYzXw@mail.gmail.com
State New
Headers show
Series
  • [ARM] Fix _unlink prototype under ARM_RDI_MONITOR.
Related show

Commit Message

Christophe Lyon Oct. 1, 2018, 9:28 p.m.
Hi,

I noticed that "path" is actually used when compiling with
ARM_RDI_MONITOR, so this small patch removes the unused attribute in
that case.

OK?

Christophe
commit 421cd7000646b4462df0195ba2d43a093fdd9851
Author: Christophe Lyon <christophe.lyon@linaro.org>
Date:   Mon Oct 1 16:15:35 2018 +0000

    [ARM] Fix _unlink prototype under ARM_RDI_MONITOR.
    
    2018-10-01  Christophe Lyon  <christophe.lyon@linaro.org>
    
    	* newlib/libc/sys/arm/syscalls.c (_unlink): Remove unused
    	attribute when compiling with ARM_RDI_MONITOR.

Comments

Eric Blake Oct. 1, 2018, 10:15 p.m. | #1
On 10/1/18 4:28 PM, Christophe Lyon wrote:
> Hi,

> 

> I noticed that "path" is actually used when compiling with

> ARM_RDI_MONITOR, so this small patch removes the unused attribute in

> that case.

> 

> OK?


Personally, I'd rather not take this patch.


>   int

> +#ifdef ARM_RDI_MONITOR

> +_unlink (const char *path)

> +#else

>   _unlink (const char *path __attribute__ ((unused)))

> +#endif


GCC (and thus clang when copying it) defined __attribute__((unused)) to 
merely mean "might be unused, so suppress warnings about it being unused 
even it if turns out to be used after all", and not "must be unused, so 
warn if it actually gets used".  It is specifically designed this way so 
that you DON'T have to add a bunch of #ifdefs around code to apply the 
attribute. If there is at least one path through the rest of a 
function's existing #ifdef maze where the attribute matters, then use 
the attribute unconditionally, rather than making the #ifdef maze worse.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Christophe Lyon Oct. 2, 2018, 6:35 a.m. | #2
On Tue, 2 Oct 2018 at 00:15, Eric Blake <eblake@redhat.com> wrote:
>

> On 10/1/18 4:28 PM, Christophe Lyon wrote:

> > Hi,

> >

> > I noticed that "path" is actually used when compiling with

> > ARM_RDI_MONITOR, so this small patch removes the unused attribute in

> > that case.

> >

> > OK?

>

> Personally, I'd rather not take this patch.

>

>

> >   int

> > +#ifdef ARM_RDI_MONITOR

> > +_unlink (const char *path)

> > +#else

> >   _unlink (const char *path __attribute__ ((unused)))

> > +#endif

>

> GCC (and thus clang when copying it) defined __attribute__((unused)) to

> merely mean "might be unused, so suppress warnings about it being unused

> even it if turns out to be used after all", and not "must be unused, so

> warn if it actually gets used".  It is specifically designed this way so

> that you DON'T have to add a bunch of #ifdefs around code to apply the

> attribute. If there is at least one path through the rest of a

> function's existing #ifdef maze where the attribute matters, then use

> the attribute unconditionally, rather than making the #ifdef maze worse.

>


OK, makes sense.

Thanks

> --

> Eric Blake, Principal Software Engineer

> Red Hat, Inc.           +1-919-301-3266

> Virtualization:  qemu.org | libvirt.org

Patch

diff --git a/newlib/libc/sys/arm/syscalls.c b/newlib/libc/sys/arm/syscalls.c
index 311b28d..6d2ad79 100644
--- a/newlib/libc/sys/arm/syscalls.c
+++ b/newlib/libc/sys/arm/syscalls.c
@@ -534,7 +534,11 @@  _link (const char *__path1 __attribute__ ((unused)),
 }
 
 int
+#ifdef ARM_RDI_MONITOR
+_unlink (const char *path)
+#else
 _unlink (const char *path __attribute__ ((unused)))
+#endif
 {
 #ifdef ARM_RDI_MONITOR
   int block[2];