This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Add a testcase for i386 LD_AUDIT
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: GNU C Library <libc-alpha at sourceware dot org>, Igor Zamyatin <igor dot zamyatin at intel dot com>
- Date: Tue, 7 Jul 2015 05:36:14 -0700
- Subject: Re: [PATCH] Add a testcase for i386 LD_AUDIT
- Authentication-results: sourceware.org; auth=none
- References: <20150704153925 dot GA14560 at gmail dot com> <20150706093356 dot GB17734 at vapier> <20150706123832 dot GA9746 at gmail dot com>
On Mon, Jul 6, 2015 at 5:38 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Jul 06, 2015 at 05:33:56AM -0400, Mike Frysinger wrote:
>> On 04 Jul 2015 08:39, H.J. Lu wrote:
>> > --- /dev/null
>> > +++ b/sysdeps/i386/tst-audit3.c
>> >
>> > +extern long long audit1_test (int, int, int) __attribute__ ((regparm(3)));
>> > +extern float audit2_test (int, int, int) __attribute__ ((regparm(3)));
>>
>> better to create a local header than copy & paste this in multiple places ?
>
> Done.
>
>>
>> > +long long
>> > +__attribute__ ((regparm(3)))
>> > +audit1_test (int i, int j, int k)
>> > +{
>> > + if (i != 1 || j != 2 || k != 3)
>>
>> only one space before k
>
> Fixed.
>
>>
>> > + abort ();
>>
>> isn't this what assert() is for ? or are you worried about compile settings ?
>>
>
> I prefer abort () here since it checks the ld.so implementation and
> it should never happen.
>
>> > +float
>> > +__attribute__ ((regparm(3)))
>> > +audit2_test (int i, int j, int k)
>> > +{
>> > + if (i != 1 || j != 2 || k != 3)
>>
>> only one space before k
>
> Fixed.
>
>> > +void
>> > +la_activity (uintptr_t *cookie, unsigned int flag)
>> > +{
>> > + if (flag == LA_ACT_CONSISTENT)
>> > + printf ("activity: consistent\n");
>> > + else if (flag == LA_ACT_ADD)
>> > + printf ("activity: add\n");
>> > + else if (flag == LA_ACT_DELETE)
>> > + printf ("activity: delete\n");
>> > + else
>> > + printf ("activity: unknown activity %u\n", flag);
>> > +}
>>
>> could do at the start:
>> printf ("activity: ");
>>
>> then make it a case statement
>>
>> > +char *
>> > +la_objsearch (const char *name, uintptr_t *cookie, unsigned int flag)
>> > +{
>> > + char buf[100];
>> > + const char *flagstr;
>> > + if (flag == LA_SER_ORIG)
>> > + flagstr = "LA_SET_ORIG";
>> > + else if (flag == LA_SER_LIBPATH)
>> > + flagstr = "LA_SER_LIBPATH";
>> > + else if (flag == LA_SER_RUNPATH)
>> > + flagstr = "LA_SER_RUNPATH";
>> > + else if (flag == LA_SER_CONFIG)
>> > + flagstr = "LA_SER_CONFIG";
>> > + else if (flag == LA_SER_DEFAULT)
>> > + flagstr = "LA_SER_DEFAULT";
>> > + else if (flag == LA_SER_SECURE)
>> > + flagstr = "LA_SER_SECURE";
>> > + else
>> > + {
>> > + sprintf (buf, "unknown flag %d", flag);
>> > + flagstr = buf;
>> > + }
>>
>> seems like a case statement would be better ?
>> -mike
>
> Here is the updated patch.
>
>
> H.J.
> ---
> From 3ea29d3f93d57116f45b189d39bc290ecfd564c2 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Sat, 4 Jul 2015 08:30:27 -0700
> Subject: [PATCH] Add a testcase for i386 LD_AUDIT
>
> This patch adds a testcase for i386 LD_AUDIT to check function return
> and parameters passed in registers.
>
> * sysdeps/i386/Makefile (tests)[elf]: Add tst-audit3.
> (modules-names): Add tst-auditmod3a tst-auditmod3b.
> ($(objpfx)tst-audit3): New rule.
> ($(objpfx)tst-audit3.out): Likewise.
> * sysdeps/i386/tst-audit3.c: New file.
> * sysdeps/i386/tst-audit3.h: Likewise.
> * sysdeps/i386/tst-auditmod3a.c: Likewise.
> * sysdeps/i386/tst-auditmod3b.c: Likewise.
I checked it in.
--
H.J.