Re: [LTP] [PATCH???v3] lib/cloner.c: add function ltp_clone7 when clone supports???7 arguments

[Jan S. <jst...@re...>]

----- Original Message -----
> From: ch...@su...
> To: "Jan Stancek" <jst...@re...>
> Cc: "Mike Frysinger" <va...@ge...>, ltp...@li...
> Sent: Thursday, 9 January, 2014 7:08:05 PM
> Subject: Re: [LTP] [PATCH???v3] lib/cloner.c: add function ltp_clone7 when clone supports???7 arguments
> 
> Hi!
> > > Hi!
> > > And here is a patch. (note that it should be applied over the two
> > > allready commited patches)
> > 
> > Hi,
> > 
> > Line 130 contains a typo:
> > clone08.c: In function ??clone_child??:
> > clone08.c:130: warning: implicit declaration of function ??tst_brmk??
> 
> Aw, I will fix that.
> 
> > In relation to:
> > #ifndef CLONE_SUPPORTS_7_ARGS
> > # define clone(fn, stack, flags, arg, ptid, tls, ctid) \
> >          clone(fn, stack, flags, arg)
> >          #endif
> > 
> > Shouldn't we have something similar also for clone2()?
> > For example:
> > 
> > --- cloner.c    2014-01-09 18:10:27.965074200 +0100
> > +++ /tmp/cloner.c       2014-01-09 18:43:28.099822061 +0100
> > @@ -34,10 +34,14 @@
> >  #undef clone                   /* we want to use clone() */
> > 
> >  #if defined(__ia64__)
> > -#define clone2 __clone2
> > +# ifndef CLONE_SUPPORTS_7_ARGS
> > +# define clone2(fn, stack, stack_size, flags, arg, ptid, tls, ctid) \
> > +         __clone2(fn, stack, stack_size, flags, arg)
> > +# else
> > +# define clone2 __clone2
> > +# endif
> >  extern int __clone2(int (*fn) (void *arg), void *child_stack_base,
> > -                   size_t child_stack_size, int flags, void *arg,
> > -                   pid_t *parent_tid, void *tls, pid_t *child_tid);
> > +                   size_t child_stack_size, int flags, void *arg, ...);
> >  #endif
> > 
> >  #ifndef CLONE_SUPPORTS_7_ARGS
> 
> I've been unable to locate ia64 distribution that does not support the
> additional parameters so I've leaved that out. It may be related to a
> fact that linux was ported to ia64 after the interface has changed, but
> that is just my guess.
> 
> And I guess that we may drop the extern int definition too, at least the
> configure script with __clone2() function seems to work fine just with
> the <sched.h>

Fair enough, I'll leave it up to you. I ran it on RHEL5 ia64 and all
looked good. The rest looks good to me:
  Reviewed-by: Jan Stancek <jst...@re...>

Regards,
Jan

> 
> --
> Cyril Hrubis
> ch...@su...
>