lkml.org 
[lkml]   [2011]   [Mar]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 1/4] exec: introduce get_arg_ptr() helper
    On 03/03, KOSAKI Motohiro wrote:
    >
    > > +static const char __user *
    > > +get_arg_ptr(const char __user * const __user *argv, int argc)
    > > +{
    >
    > [argc, argv] is natural order to me than [argv, argc].

    Yes... in fact, "argc" is misnamed here. It doesn't mean the number of
    arguments, it is the index in the array. Perhaps this should be [argv, nr].

    > and "get_" prefix are usually used for reference count incrementing
    > function in linux. so, i _personally_ prefer to call "user_arg_ptr".

    Agreed, the name is ugly. I'll rename and resend keeping your reviewed-by.

    [2/4]
    > I _personally_ don't like "conditional". Its name is based on code logic.
    > It's unclear what mean "conditional". From data strucuture view, It is
    > "opaque userland pointer".

    I agree with any naming, just suggest a better name ;)

    [3/4]
    > > + struct conditional_ptr argv = {
    > > + .is_compat = true, .ptr.compat = __argv,
    > > + };
    >
    > Please don't mind to compress a line.
    >
    > struct conditional_ptr argv = {
    > .is_compat = true,
    > .ptr.compat = __argv,
    > };

    OK, will do.

    Thanks for review!

    Oleg.



    \
     
     \ /
      Last update: 2011-03-03 16:59    [W:0.022 / U:0.208 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site