Re: [PATCH 1/2] sg: factor out sg_fill_request_table()

[Douglas Gilbert]

On 2017-09-15 08:05 AM, Hannes Reinecke wrote:

Factor our sg_fill_request_table() for better readability.

Signed-off-by: Hannes Reinecke <h...@suse.com>

Acked-by: Douglas Gilbert <dgilb...@interlog.com>

Thanks.

---
  drivers/scsi/sg.c | 62 ++++++++++++++++++++++++++++++++-----------------------
  1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 03194c4..f1e20ca0 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -857,6 +857,40 @@ static int max_sectors_bytes(struct request_queue *q)
        return max_sectors << 9;
  }

+static void

+sg_fill_request_table(Sg_fd *sfp, sg_req_info_t *rinfo)
+{
+       Sg_request *srp;
+       int val;
+       unsigned int ms;
+
+       val = 0;
+       list_for_each_entry(srp, &sfp->rq_list, entry) {
+               if (val > SG_MAX_QUEUE)
+                       break;
+               memset(&rinfo[val], 0, SZ_SG_REQ_INFO);
+               rinfo[val].req_state = srp->done + 1;
+               rinfo[val].problem =
+                       srp->header.masked_status &
+                       srp->header.host_status &
+                       srp->header.driver_status;
+               if (srp->done)
+                       rinfo[val].duration =
+                               srp->header.duration;
+               else {
+                       ms = jiffies_to_msecs(jiffies);
+                       rinfo[val].duration =
+                               (ms > srp->header.duration) ?
+                               (ms - srp->header.duration) : 0;
+               }
+               rinfo[val].orphan = srp->orphan;
+               rinfo[val].sg_io_owned = srp->sg_io_owned;
+               rinfo[val].pack_id = srp->header.pack_id;
+               rinfo[val].usr_ptr = srp->header.usr_ptr;
+               val++;
+       }
+}
+
  static long
  sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
  {
@@ -1041,39 +1075,15 @@ static int max_sectors_bytes(struct request_queue *q)
                        return -EFAULT;
                else {
                        sg_req_info_t *rinfo;
-                       unsigned int ms;

rinfo = kmalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,

                                                                GFP_KERNEL);
                        if (!rinfo)
                                return -ENOMEM;
                        read_lock_irqsave(&sfp->rq_list_lock, iflags);
-                       val = 0;
-                       list_for_each_entry(srp, &sfp->rq_list, entry) {
-                               if (val > SG_MAX_QUEUE)
-                                       break;
-                               memset(&rinfo[val], 0, SZ_SG_REQ_INFO);
-                               rinfo[val].req_state = srp->done + 1;
-                               rinfo[val].problem =
-                                       srp->header.masked_status &
-                                       srp->header.host_status &
-                                       srp->header.driver_status;
-                               if (srp->done)
-                                       rinfo[val].duration =
-                                               srp->header.duration;
-                               else {
-                                       ms = jiffies_to_msecs(jiffies);
-                                       rinfo[val].duration =
-                                               (ms > srp->header.duration) ?
-                                               (ms - srp->header.duration) : 0;
-                               }
-                               rinfo[val].orphan = srp->orphan;
-                               rinfo[val].sg_io_owned = srp->sg_io_owned;
-                               rinfo[val].pack_id = srp->header.pack_id;
-                               rinfo[val].usr_ptr = srp->header.usr_ptr;
-                               val++;
-                       }
+                       sg_fill_request_table(sfp, rinfo);
                        read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
+
                        result = __copy_to_user(p, rinfo,
                                                SZ_SG_REQ_INFO * SG_MAX_QUEUE);
                        result = result ? -EFAULT : 0;