2nd attempt.

Partially implemented recommendations.
- Still using xxc_p (where xx can stand for sm, mm, ss etc for smc, mmc, 
ssc module). Better name welcome.
- I have not addressed the 'device_type_datasize' in the 
device_type_template within this patch.


 From 5ed6c1003784ddda85068f81c6a9a34937f4c58c Mon Sep 17 00:00:00 2001
From: Mark Harvey <[EMAIL PROTECTED]>
Date: Thu, 12 Jun 2008 06:52:22 +1000
Subject: Remove duplicate pointers.

Share a common pointer in scsi_lu for modules private use.

Signed-off-by: Mark Harvey <[EMAIL PROTECTED]>
---
  usr/mmc.c  |   52 ++++++++++++++++++++++++++--------------------------
  usr/smc.c  |   14 +++++++-------
  usr/tgtd.h |   10 ++++++----
  3 files changed, 39 insertions(+), 37 deletions(-)

diff --git a/usr/mmc.c b/usr/mmc.c
index f988f5a..0e34167 100644
--- a/usr/mmc.c
+++ b/usr/mmc.c
@@ -59,7 +59,7 @@ struct mmc_info {

  static int mmc_rw(int host_no, struct scsi_cmd *cmd)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        int ret;
        uint64_t end_offset;
        uint64_t offset, length;
@@ -134,7 +134,7 @@ static int mmc_rw(int host_no, struct scsi_cmd *cmd)

  static int mmc_read_capacity(int host_no, struct scsi_cmd *cmd)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        uint64_t size;
        uint32_t *data;

@@ -164,7 +164,7 @@ overflow:

  static int mmc_read_toc(int host_no, struct scsi_cmd *cmd)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        uint8_t *data;
        uint8_t buf[32];
        int toc_time, toc_format, toc_track;
@@ -282,7 +282,7 @@ static int mmc_read_toc(int host_no, struct scsi_cmd 
*cmd)

  static int mmc_close_track(int host_no, struct scsi_cmd *cmd)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);

        /* once we close the track it becomes a DVD_ROM */
        mmc->current_profile = PROFILE_DVD_ROM;
@@ -292,7 +292,7 @@ static int mmc_close_track(int host_no, struct 
scsi_cmd *cmd)

  static int mmc_read_disc_information(int host_no, struct scsi_cmd *cmd)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        unsigned char buf[34];

        if (mmc->current_profile == PROFILE_NO_PROFILE) {
@@ -460,7 +460,7 @@ static int mmc_read_disc_information(int host_no, 
struct scsi_cmd *cmd)

  static void profile_dvd_rom(struct scsi_cmd *cmd, char *data)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);

        /* profile number */
        *data++ = 0;
@@ -478,7 +478,7 @@ static void profile_dvd_rom(struct scsi_cmd *cmd, 
char *data)

  static void profile_dvd_plus_r(struct scsi_cmd *cmd, char *data)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);

        /* profile number */
        *data++ = 0;
@@ -634,7 +634,7 @@ static char *feature_removable_medium(struct 
scsi_cmd *cmd, char *data,
  static char *feature_random_readable(struct scsi_cmd *cmd, char *data,
                                     int only_current)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        int is_current;

        /* this feature is only current in DVD_ROM */
@@ -684,7 +684,7 @@ static char *feature_random_readable(struct scsi_cmd 
*cmd, char *data,
  static char *feature_dvd_read(struct scsi_cmd *cmd, char *data,
                              int only_current)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        int is_current;

        /* this feature is only current in DVD_ROM */
@@ -749,7 +749,7 @@ static char *feature_timeout(struct scsi_cmd *cmd, 
char *data, int only_current)
  static char *feature_real_time_streaming(struct scsi_cmd *cmd, char *data,
                                         int only_current)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        int is_current;

        /* this feature is only current in DVD_ROM */
@@ -791,7 +791,7 @@ static char *feature_real_time_streaming(struct 
scsi_cmd *cmd, char *data,
  static char *feature_dvd_plus_r(struct scsi_cmd *cmd, char *data,
                                int only_current)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        int is_current;

        /* this feature is only current in DVD+R */
@@ -875,7 +875,7 @@ static char *feature_multi_read(struct scsi_cmd 
*cmd, char *data,

  static char *feature_dcbs(struct scsi_cmd *cmd, char *data, int 
only_current)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        int is_current;

        /* this feature is only current in DVD+R */
@@ -961,7 +961,7 @@ struct feature_descriptor features[] = {

  static int mmc_get_configuration(int host_no, struct scsi_cmd *cmd)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        char *data;
        char buf[1024];
        int rt, start;
@@ -1017,7 +1017,7 @@ static int mmc_get_configuration(int host_no, 
struct scsi_cmd *cmd)
  static unsigned char *track_type_lba(struct scsi_cmd *cmd, unsigned 
char *data,
                                     unsigned int lba)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        unsigned long tmp;

        switch (mmc->current_profile) {
@@ -1170,7 +1170,7 @@ static unsigned char *track_type_lba(struct 
scsi_cmd *cmd, unsigned char *data,
  static unsigned char *track_type_track(struct scsi_cmd *cmd,
                                       unsigned char *data, unsigned int lba)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        unsigned long tmp;

        switch (mmc->current_profile) {
@@ -1352,7 +1352,7 @@ struct track_type track_types[] = {

  static int mmc_read_track_information(int host_no, struct scsi_cmd *cmd)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        struct track_type *t;
        unsigned char *data;
        unsigned char buf[4096];
@@ -1402,7 +1402,7 @@ static int mmc_read_track_information(int host_no, 
struct scsi_cmd *cmd)

  static int mmc_read_buffer_capacity(int host_no, struct scsi_cmd *cmd)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        int blocks;
        unsigned char buf[12];
        long tmp;
@@ -1457,7 +1457,7 @@ static int mmc_read_buffer_capacity(int host_no, 
struct scsi_cmd *cmd)

  static int mmc_synchronize_cache(int host_no, struct scsi_cmd *cmd)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);

        if (mmc->current_profile == PROFILE_NO_PROFILE) {
                scsi_set_in_resid_by_actual(cmd, 0);
@@ -1473,7 +1473,7 @@ static unsigned char *perf_type_write_speed(struct 
scsi_cmd *cmd,
                                            unsigned int type,
                                            unsigned int data_type)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);

        /* write/except */
        *data++ = 0x00;
@@ -1628,7 +1628,7 @@ static unsigned char *perf_type_perf_data(struct 
scsi_cmd *cmd,
                                          unsigned int type,
                                          unsigned int data_type)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        int tolerance;
        int write_flag;
        int except;
@@ -1786,7 +1786,7 @@ static unsigned char *dvd_format_phys_info(struct 
scsi_cmd *cmd,
                                           unsigned char *data, int format,
                                           int layer, int write_header)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        unsigned char *old_data;

        if (write_header) {
@@ -1937,7 +1937,7 @@ static unsigned char *dvd_format_adip_info(struct 
scsi_cmd *cmd,
                                           unsigned char *data, int format,
                                           int layer, int write_header)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);

        if (write_header) {
                *data++ = DVD_FORMAT_ADIP_INFO;
@@ -2107,7 +2107,7 @@ static unsigned char 
*dvd_format_dvd_structure_list(struct scsi_cmd *cmd,

  static int mmc_read_dvd_structure(int host_no, struct scsi_cmd *cmd)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        long address;
        int format, layer;
        unsigned char *data;
@@ -2159,7 +2159,7 @@ static int mmc_read_dvd_structure(int host_no, 
struct scsi_cmd *cmd)

  static int mmc_reserve_track(int host_no, struct scsi_cmd *cmd)
  {
-       struct mmc_info *mmc = (struct mmc_info *)cmd->dev->mmc_p;
+       struct mmc_info *mmc = dtype_priv(cmd->dev);
        uint64_t tmp;

        tmp = cmd->scb[5];
@@ -2200,7 +2200,7 @@ static int mmc_lu_init(struct scsi_lu *lu)
        if (!mmc)
                return -ENOMEM;

-       lu->mmc_p = mmc;
+       lu->xxc_p = mmc;

        if (spc_lu_init(lu))
                return TGTADM_NOMEM;
@@ -2268,7 +2268,7 @@ static int mmc_lu_init(struct scsi_lu *lu)

  static int mmc_lu_online(struct scsi_lu *lu)
  {
-       struct mmc_info *mmc = (struct mmc_info *)lu->mmc_p;
+       struct mmc_info *mmc = dtype_priv(lu);
        struct stat st;

        mmc->current_profile = PROFILE_NO_PROFILE;
diff --git a/usr/smc.c b/usr/smc.c
index 9a46d30..9d7f681 100644
--- a/usr/smc.c
+++ b/usr/smc.c
@@ -232,7 +232,7 @@ static int build_element_descriptors(uint8_t *data, 
struct list_head *head,
   */
  static int smc_read_element_status(int host_no, struct scsi_cmd *cmd)
  {
-       struct smc_info *smc = (struct smc_info *)cmd->dev->smc_p;
+       struct smc_info *smc = dtype_priv(cmd->dev);
        uint8_t *data = NULL;
        uint8_t *scb;
        uint8_t element_type;
@@ -354,7 +354,7 @@ sense:
   */
  static int smc_move_medium(int host_no, struct scsi_cmd *cmd)
  {
-       struct smc_info *smc = (struct smc_info *)cmd->dev->smc_p;
+       struct smc_info *smc = dtype_priv(cmd->dev);
        uint8_t *scb;
        uint16_t src;
        uint16_t dest;
@@ -435,7 +435,7 @@ static int smc_lu_init(struct scsi_lu *lu)

        smc = zalloc(sizeof(struct smc_info));
        if (smc)
-               lu->smc_p = smc;
+               dtype_priv(lu) = smc;
        else
                return -ENOMEM;

@@ -473,7 +473,7 @@ static int smc_lu_init(struct scsi_lu *lu)

  static void smc_lu_exit(struct scsi_lu *lu)
  {
-       struct smc_info *smc = lu->smc_p;
+       struct smc_info *smc = dtype_priv(lu);

        dprintf("Medium Changer shutdown() called\n");

@@ -547,7 +547,7 @@ static void slot_dump(struct list_head *head)

  static int add_slt(struct scsi_lu *lu, struct tmp_param *tmp)
  {
-       struct smc_info *smc = lu->smc_p;
+       struct smc_info *smc = dtype_priv(lu);
        int ret = TGTADM_INVALID_REQUEST;
        struct mode_pg *pg;
        struct slot *s;
@@ -606,7 +606,7 @@ dont_do_slots:

  static int config_slot(struct scsi_lu *lu, struct tmp_param *tmp)
  {
-       struct smc_info *smc = lu->smc_p;
+       struct smc_info *smc = dtype_priv(lu);
        struct mode_pg *m = NULL;
        struct slot *s = NULL;
        int ret = TGTADM_INVALID_REQUEST;
@@ -649,7 +649,7 @@ static int config_slot(struct scsi_lu *lu, struct 
tmp_param *tmp)

  static int __smc_lu_config(struct scsi_lu *lu, char *params)
  {
-       struct smc_info *smc = (struct smc_info *)lu->smc_p;
+       struct smc_info *smc = dtype_priv(lu);
        int err = TGTADM_SUCCESS;
        char *p;
        char buf[256];
diff --git a/usr/tgtd.h b/usr/tgtd.h
index 2f128f6..7cc1137 100644
--- a/usr/tgtd.h
+++ b/usr/tgtd.h
@@ -24,6 +24,8 @@
  #define _TAB3 _TAB1 _TAB1 _TAB1
  #define _TAB4 _TAB2 _TAB2

+#define dtype_priv(lu) (lu)->xxc_p
+
  enum tgt_system_state {
        TGT_SYSTEM_OFFLINE = 1,
        TGT_SYSTEM_READY,
@@ -148,10 +150,10 @@ struct scsi_lu {

        struct lu_phy_attr attrs;

-       /* TODO: needs a structure for lots of device parameters */
-       /* Currently only used by smc and mmc modules */
-       void *smc_p;
-       void *mmc_p;
+       /* A pointer for each modules private use.
+        * Currently used by smc and mmc modules.
+        */
+       void *xxc_p;
  };

  struct mgmt_req {
-- 
1.5.2.5


FUJITA Tomonori wrote:
> On Sun, 01 Jun 2008 11:33:22 +1000
> Mark Harvey <[EMAIL PROTECTED]> wrote:
> 
>>  From 72e5a96969937bc68f9153d5c18af306827cf254 Mon Sep 17 00:00:00 2001
>> From: Mark Harvey <[EMAIL PROTECTED]>
>> Date: Fri, 30 May 2008 17:54:16 +1000
>> Subject: Share common 'private' pointer.
>>
>> No need for smc & mmc to use different pointer.
>> Use common pointer for private use.
> 
> Yeah, cleaning them up is nice but I like to do it a bit differently.
> 
> How about adding macro like device_type_priv or dtype_priv?
> 
> #define device_type_priv(lu) (lu)->xxc_p
> 
> Then we can convert smc and mmc to use it.
> 
> And how about explanatory names like device_type_data rather than
> xxc_p?
> 
> I'm also fine with adding something like device_type_datasize to
> struct device_type_template to remove the allocation from device type
> modules.
> 
> Thanks,
> 

_______________________________________________
Stgt-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/stgt-devel

Reply via email to