On 09/04, Roland McGrath wrote:
>
> > Add "int options" into struct ptrace_context. Will be used to hold
> > PT_XXX options. Currently is not used, but:
>
> IMHO the traditional PT_* bit assignments are useless.
> We should just store and use the PTRACE_O_* bits directly.
Ah. Why, why I have not thought of it?
> > - introduce __ptrace_set_options() helper which updates ->options
> > and utrace events mask.
>
> Despite overly-common kernel practice, __foo is a lousy name for anything.
> The only place where I think __foo makes sense for new code is when foo is
> a wrapper that takes a lock around calling __foo. I'd prefer if we can
> just pick meaningful names for new internal functions. __ names are
> especially pointless for static functions.
Agreed... ptrace_set_events? I agree with any naming.
> > +static inline
> > +struct ptrace_context *ptrace_context(struct utrace_engine *engine)
> > +{
> > + return engine->data;
> > +}
>
> When the prototype doesn't fit on a line, the usual way to break it is:
>
> static inline struct ptrace_context *
> ptrace_context(struct utrace_engine *engine)
OK.
> But anyway, what's the point of this helper?
> What's wrong with or any less clear about plain:
>
> struct ptrace_context *ctx = engine->data;
>
> ?
Nothing wrong. I added this trivial helper just in case, if we change
the lifetime rules for engine->data. I will keep it for now, but we
can kill it at any moment.
Oleg.
