[Stgt-devel] [PATCH 1/1] Remove duplicate pointer.

Mark Harvey markh794
Thu Jun 12 12:13:17 CEST 2008


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 <markh794 at gmail.com>
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 <markh794 at gmail.com>
---
  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 <markh794 at gmail.com> wrote:
> 
>>  From 72e5a96969937bc68f9153d5c18af306827cf254 Mon Sep 17 00:00:00 2001
>> From: Mark Harvey <markh794 at gmail.com>
>> 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,
> 




More information about the stgt mailing list