[patch] SG_IO ioctl

Vladimir Nadvornik nadvornik at suse.cz
Tue Mar 27 13:53:07 UTC 2007


On Friday 23 March 2007 00:09, Dmitry V. Levin wrote:
> Hi,
>
> On Mon, Mar 19, 2007 at 06:00:17PM +0100, Vladimir Nadvornik wrote:
> > On Thursday 15 March 2007 02:11, Dmitry V. Levin wrote:
>
> [...]
>
> > Thank you for feedback. Please see the updated patch.
>
> Thank you for the patch.
>
> I attached my edition of this patch.  Besides of whitespace difference,
> there are 5 changes:
> + scsi_ioctl() prototype is added to defs.h;
> + buffer address is printed not only when umoven() fails but also when
> malloc() fails, for consistency;
> + malloc/umoven/free code is incapsulated into print_sg_io_buffer()
> function;
> + in print_sg_io_buffer(), max_strlen limit is applied;
> + format string for flags, host_status, driver_status, and info fields is
> changed from %0Nx to more general %#x.
>
> Please have a look.

Looks OK.

>
> > > > +		if (exiting(tcp)) {
> > > > +			struct sg_io_hdr sg_io;
> > > > +			if (umove(tcp, arg, &sg_io) >= 0) {
> > > > +				print_sg_io_res(tcp, &sg_io);
> > > > +			}
> > > > +		}
> > >
> > > Looks like syserror(tcp) check is missing here.
> >
> > If there is an error, it prints the content of the structure as it was
> > passed in. I think that it is acceptable behavior and special handling
> > is not necessary.
>
> I still not sure this is right way to do.
> It depends whether kernel fills "output" part of struct sg_io_hdr in case
> of an error.  If it does, then it should be parsed, otherwise there are no
> reason to output it.

OK, I have changed it, see the attachment.


Vladimir Nadvornik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: strace-ioctl-scsi.diff
Type: text/x-diff
Size: 4074 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20070327/9f9dd601/attachment.bin>


More information about the Strace-devel mailing list