[Stgt-devel] PATCH 4 of 6 -

Ming Zhang blackmagic02881
Fri May 11 16:20:02 CEST 2007


On Fri, 2007-05-11 at 13:19 +1000, Mark Harvey wrote:

<snip>

> + */
> +#include <errno.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <linux/fs.h>
> +
> +#include "list.h"
> +#include "util.h"
> +#include "tgtd.h"
> +#include "target.h"
> +#include "driver.h"
> +#include "tgtadm.h"
> +#include "scsi.h"
> +#include "spc.h"
> +#include "ssc3.h"
> +#include "sense_codes.h"
> +#include "media.h"
> +

pls double check if all header files are needed here.

> +/* *********************************************
> + * Useful routines for SCSI OP Code processing
> + * *********************************************/
> +static struct physicalAttributes *
> +getPhyAttr(struct scsi_cmd *cmd)
> +{
> +       struct ssc_info * ssc = cmd->dev->priv_p;
> +       return ssc->phy;
> +}

make this inline?

> +
> +static int
> +check_reset(struct physicalAttributes * phy)
> +{
> +       return phy->reset;
> +}

inline again?

> +
> +/* *********************************************
> + * SCSI OP Code processing
> + * *********************************************/

comment like this can be omitted.


> +static int
> +ssc_TUR(int host_no, struct scsi_cmd *cmd)
> +{
> +       int ret = SAM_STAT_GOOD;
> +       struct physicalAttributes * phy = getPhyAttr(cmd);
> +
> +       cmd->len = 0;
> +       if (cmd->dev) {
> +               ret = device_reserved(cmd);
> +               if (ret) {
> +                       dprintf("Reservation Conflict");
> +                       return SAM_STAT_RESERVATION_CONFLICT;
> +               }
> +               if(check_reset(phy)) {
> +                       dprintf("Power-on or reset");
> +                       mk_sense_data(cmd, NOT_READY,
> E_POWERON_RESET);
> +                       ret = SAM_STAT_CHECK_CONDITION;
> +                       phy->reset = 0;
> +               } else if (! phy->onLine) {
> +                       dprintf("No media in drive");
> +                       mk_sense_data(cmd, NOT_READY,
> E_MEDIUM_NOT_PRESENT);
> +                       ret = SAM_STAT_CHECK_CONDITION;
> +               }
> +       } else {
> +               dprintf("Invalid field in cdb");
> +               mk_sense_data(cmd, ILLEGAL_REQUEST,
> E_INVALID_FIELD_IN_CDB);
> +               ret = SAM_STAT_CHECK_CONDITION;
> +       }
> +       return ret;
> +}
> +
> +static int
> +ssc_inquiry(int host_no, struct scsi_cmd *cmd)
> +{
> +       int len, ret = SAM_STAT_CHECK_CONDITION;
> +       uint8_t         *data;
> +       uint8_t         *scb = cmd->scb;
> +       unsigned char   device_type =
> cmd->c_target->dev_type_template.type;
> +       unsigned char   key = ILLEGAL_REQUEST;
> +       uint16_t        asc_ascq = E_INVALID_FIELD_IN_CDB;
> +       struct physicalAttributes * phy = getPhyAttr(cmd);
> +
> +       if (((scb[1] & 0x3) == 0x3) || (!(scb[1] & 0x3) && scb[2]))
> +               goto sense;
> +
> +       data = valloc(pagesize);
> +       if (!data) {
> +               key = HARDWARE_ERROR;
> +               asc_ascq = E_INTERNAL_TARGET_FAILURE;
> +               goto sense;
> +       }
> +       memset(data, 0, pagesize);
> +
> +       dprintf("%x %x, device_type: %d\n", scb[1], scb[2],
> device_type);
> +
> +       if (!(scb[1] & 0x3)) {
> +               int tmp = 62;

tmp set to hex or decimal in different places, could u use only one
format? ;)


> +               data[0] = device_type;
> +               data[1] = 0x80;         // Removable
> +               data[2] = 4;            // Conform to SCSI-3
> +               data[3] = tmp;
> +               data[4] = 59;
> +               data[7] = 0x02;
> +               memset(data + 8, 0x20, 28);
> +               memcpy((char *)data + 8, phy->VendorIdent, 8);
> +               memcpy((char *)data + 16, phy->ProductIdent, 16);
> +               memcpy((char *)data + 32, phy->ProductRev, 4);
> +               data[58] = 0x03;        // 0300 == No SPC-3 version
> claimed
> +               data[59] = 0x00;
> +               data[60] = 0x09;        // 0960 == iSCSI
> +               data[61] = 0x60;
> +               data[62] = 0x02;        // 0200 == No SCC version
> claimed
> +               data[63] = 0x00;
> +               data[64] = 0x04;        // 0400 == No SCC-3 version
> claimed
> +               data[65] = 0x00;
> +               len = tmp + 4;
> +               ret = SAM_STAT_GOOD;
> +       } else if (scb[1] & 0x2) {
> +               /* CmdDt bit is set */
> +               /* We do not support it now. */
> +               data[1] = 0x1;
> +               data[5] = 0;
> +               len = 6;
> +               ret = SAM_STAT_GOOD;
> +       } else if (scb[1] & 0x1) {
> +               /* EVPD bit set */
> +               if (scb[2] == 0x0) {
> +                       data[0] = device_type;
> +                       data[1] = 0x0;
> +                       data[3] = 6;
> +                       data[4] = 0x0;  // Return supported pages
> +                       data[5] = 0x80; // Serial Number
> +                       data[6] = 0x83; // Device Identification
> +                       data[7] = 0xb0; // SSC VPD
> +                       data[8] = 0xc0; // Firmware build
> +                       data[9] = 0xc1; // Subsystem Revision
> +                       len = 10;
> +                       ret = SAM_STAT_GOOD;
> +               } else if (scb[2] == 0x80) {
> +                       int tmp = SCSI_SN_LEN;
> +
> +                       data[1] = 0x80;
> +                       data[3] = SCSI_SN_LEN;
> +                       memset(data + 4, 0x20, 4);
> +                       len = 4 + SCSI_SN_LEN;
> +                       ret = SAM_STAT_GOOD;
> +
> +                       if (cmd->dev && strlen(phy->SerialNumber)) {
> +                               uint8_t *p;
> +                               char *q;
> +
> +                               p = data + 4 + tmp - 1;
> +                               q = phy->SerialNumber + SCSI_SN_LEN -
> 1;
> +
> +                               for (; tmp > 0; tmp--, q)
> +                                       *(p--) = *(q--);
> +                       }
> +               } else if (scb[2] == 0x83) {
> +                       int tmp = 40 + SCSI_SN_LEN;
> +
> +                       data[1] = 0x83;
> +                       data[3] = tmp + 4;
> +                       data[4] = 0x1;
> +                       data[5] = 0x1;
> +                       data[7] = tmp;
> +                       if (cmd->dev) {
> +                               strncpy((char *) data + 8,
> +                                               cmd->dev->scsi_id,
> +                                               SCSI_ID_LEN);
> +                               data[36] = 2;   // ASCII
> +                               data[37] = 1;
> +                               data[39] = SCSI_SN_LEN;
> +                               strncpy((char *) data + 40,
> +                                               phy->SerialNumber,
> +                                               SCSI_SN_LEN);
> +                       }
> +                       len = tmp + 8;
> +                       ret = SAM_STAT_GOOD;
> +               } else if (scb[2] == 0xb0) {    // SSC VPD
> +                       int tmp = 3;
> +                       data[1] = 0xb0;
> +                       data[2] = 0;    // Length - MSB
> +                       data[3] = tmp;  // Length - LSB
> +                       data[4] = 1;    // WORM support
> +                       len = tmp + 4;
> +                       ret = SAM_STAT_GOOD;
> +               } else if (scb[2] == 0xc0) {    // Firmware Build
> +                       int tmp = 0x20;
> +                       data[1] = 0xc0;
> +                       data[2] = 0;    // Length - MSB
> +                       data[3] = tmp;  // Length - LSB
> +                       strncpy((char *) data + 20, "2007-03-10
> 05:30", 20);
> +                       len = tmp + 4;
> +                       ret = SAM_STAT_GOOD;
> +               } else if (scb[2] == 0xc1) {    // Subsystem Revision
> +                       int tmp = 4;
> +                       data[1] = 0xc1;
> +                       data[2] = 0;    // Length - MSB
> +                       data[3] = tmp;  // Length - LSB
> +                       strncpy((char *) data + 4, "blah", 4);
> +                       len = tmp + 4;
> +                       ret = SAM_STAT_GOOD;
> +               }
> +       }
> +
> +       if (ret != SAM_STAT_GOOD)
> +               goto sense;

by goto sense u will have a memory leak here. data is not freed. seems
other places have this problem as well.

<snip>

others looks ok to me.

> _______________________________________________
> Stgt-devel mailing list
> Stgt-devel at lists.berlios.de
> https://lists.berlios.de/mailman/listinfo/stgt-devel
> 




More information about the stgt mailing list