On Wed, 22 Jun 2016 13:37:19 +0300
"Dmitry V. Levin" <l...@altlinux.org> wrote:

> On Wed, Jun 22, 2016 at 11:30:29AM +0200, Antoine Damhet wrote:
> [...]
> > > > +               if (_IOC_SIZE(type) > 0) {
> > > > +                       tprints("[");
> > > > +                       printxval(x, type, str);
> > > > +                       tprints(", ");
> > > > +                       if (pos + sizeof(type) +
> > > > _IOC_SIZE(type) <= size)
> > > > +                               printstr(tcp, buffer + pos +
> > > > sizeof(type),
> > > > +
> > > > _IOC_SIZE(type));
> > > > +                       tprints("]");    
> > > 
> > > What's going to be printed when this condition is false?  
> > 
> > What should I print ?  
> 
> The same as in the case of _IOC_SIZE(type) == 0?
> 
> > > > +               } else
> > > > +                       printxval(x, type, str);
> > > > +               pos += sizeof(uint32_t) + _IOC_SIZE(type);    
> > > 
> > > What's going to happen when pos overflows?  
> > 
> > The umove should have failed before but should I still check it ?  
> 
> As type contains an arbitrary value just fetched by umove,
> it has to be checked.
> 
> > > > +       }
> > > > +
> > > > +       tprints("]");
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int
> > > > +decode_binder_write_read(struct tcb *tcp, const long addr)
> > > > +{
> > > > +       struct binder_write_read wr;
> > > > +
> > > > +       if (entering(tcp)) {
> > > > +               tprints(", ");
> > > > +               if (umove_or_printaddr(tcp, addr, &wr))
> > > > +                       return RVAL_DECODED | 1;
> > > > +
> > > > +               tprintf("{write_size=%" PRIu64 ",
> > > > write_consumed=%" PRIu64
> > > > +                               ", write_buffer=",
> > > > +                               (uint64_t)wr.write_size,
> > > > +                               (uint64_t)wr.write_consumed);
> > > > +               if (decode_binder_commands_buffer(tcp,
> > > > wr.write_buffer,
> > > > +                                       wr.write_consumed,
> > > > wr.write_size,
> > > > +                                       binder_driver_commands,
> > > > "BC_???")) {    
> > > 
> > > Does the kernel take write_consumed into account?
> > > If not, shouldn't 0 be passed to decode_binder_commands_buffer
> > > instead, and what's the use of printing write_consumed on
> > > entering?  
> > 
> > Yes, the kernel take write_consumed into account.
> >   
> > > > +                       tprints("}");
> > > > +                       return RVAL_DECODED | 1;
> > > > +               }
> > > > +
> > > > +               tprintf(", read_size=%" PRIu64 ",
> > > > read_consumed=%" PRIu64 "}",
> > > > +                               (uint64_t)wr.read_size,
> > > > +
> > > > (uint64_t)wr.read_consumed);    
> > > 
> > > If read_consumed is write only, what's the use of printing it on
> > > entering?  
> > 
> > It's not but I'm not sure how I should keep read_consumed between
> > entering and exiting.  
> 
> Since write_consumed and read_consumed are read-write,
> it's OK to print them twice as you did.
> 
> > > [...]  
> > > > --- a/configure.ac
> > > > +++ b/configure.ac
> > > > @@ -440,6 +440,24 @@ AC_CHECK_HEADERS([linux/bpf.h], [
> > > >         fi
> > > >  ])
> > > >  
> > > > +AC_CHECK_HEADERS([linux/android/binder.h], [[ #include
> > > > <sys/types.h> +#include <linux/android/binder.h>])
> > > > +        AC_CHECK_DECLS(m4_normalize([BC_TRANSACTION, BC_REPLY,
> > > > BC_ACQUIRE_RESULT,
> > > > +                               BC_FREE_BUFFER, BC_INCREFS,
> > > > BC_ACQUIRE,
> > > > +                               BC_RELEASE, BC_DECREFS,
> > > > BC_INCREFS_DONE,
> > > > +                               BC_ACQUIRE_DONE,
> > > > BC_ATTEMPT_ACQUIRE,
> > > > +                               BC_REGISTER_LOOPER,
> > > > BC_ENTER_LOOPER,
> > > > +                               BC_EXIT_LOOPER,
> > > > BC_REQUEST_DEATH_NOTIFICATION,
> > > > +                               BC_CLEAR_DEATH_NOTIFICATION,
> > > > BC_DEAD_BINDER_DONE,
> > > > +                               BR_ERROR, BR_OK,
> > > > BR_TRANSACTION, BR_REPLY,
> > > > +                               BR_ACQUIRE_RESULT,
> > > > BR_DEAD_REPLY,
> > > > +                               BR_TRANSACTION_COMPLETE,
> > > > BR_INCREFS, BR_ACQUIRE,
> > > > +                               BR_RELEASE, BR_DECREFS,
> > > > BR_ATTEMPT_ACQUIRE,
> > > > +                               BR_NOOP, BR_SPAWN_LOOPER,
> > > > BR_FINISHED,
> > > > +                               BR_DEAD_BINDER,
> > > > BR_CLEAR_DEATH_NOTIFICATION_DONE,
> > > > +                               BR_FAILED_REPLY]),,,[ #include
> > > > <sys/types.h>    
> > > 
> > > Please list these constants sorted in one column like in other
> > > places.  
> > 
> > Will do.
> >   
> > > > +#include <linux/android/binder.h>])])  
> > > 
> > > I'm not quite sure what's expected from defined(__ANDROID__) in
> > > binder.c and whether this check is consistent with conditions
> > > where <linux/android/binder.h> is not available but
> > > defined(__ANDROID__) is true.  
> > 
> > On android, the header is located at <linux/binder.h> and since
> > it's a vital component in the system so it will be there.  
> 
> Does #include <linux/android/binder.h> work on android?
> If not, how this check is expected to work on android?

at the moment, #include <linux/android/binder.h> will not work on
android.

Since android does not use the configure, but a separate Makefile with
all the values hardcoded, does it makes sense to bother with it?

-- 
Gabriel Laskar

------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to