Skip to content

Use vector for INQUIRY data, LUN list can have gaps, made methods const#713

Merged
akuker merged 59 commits intodevelopfrom
feature_inquiry_cleanup
Mar 2, 2022
Merged

Use vector for INQUIRY data, LUN list can have gaps, made methods const#713
akuker merged 59 commits intodevelopfrom
feature_inquiry_cleanup

Conversation

@uweseimet
Copy link
Copy Markdown
Contributor

@uweseimet uweseimet commented Feb 27, 2022

Inquiry() returns vector instead of accepting BYTE * and is declared const. This reduces the risk of memory issues and side-effects.
All devices make use of PrimaryDevice::Inquiry(), which elimates code duplication.
Moved error codes to existing scsi_defs namespace.
Fixed ReadDefectData10.
Do not use sorted maps/sets where not needed.
The LUNs list can have gaps, but LUN 0 is mandatory as long as there is also another LUN.

@uweseimet uweseimet marked this pull request as draft February 27, 2022 22:15
@uweseimet uweseimet requested a review from rdmark February 27, 2022 22:15
@uweseimet uweseimet marked this pull request as ready for review February 27, 2022 23:28
@uweseimet uweseimet marked this pull request as draft February 28, 2022 07:24
@uweseimet uweseimet linked an issue Feb 28, 2022 that may be closed by this pull request
@uweseimet uweseimet marked this pull request as ready for review February 28, 2022 07:44
@erichelgeson erichelgeson removed their request for review February 28, 2022 14:36
@uweseimet uweseimet changed the title Use vector for INQUIRY data, Inquiry() is const, moved EVPD check Use vector for INQUIRY data, Made Inquiry() and other methods const Feb 28, 2022
@uweseimet uweseimet changed the title Use vector for INQUIRY data, Made Inquiry() and other methods const Use vector for INQUIRY data, made Inquiry() and other methods const Feb 28, 2022
@uweseimet uweseimet linked an issue Mar 1, 2022 that may be closed by this pull request
@uweseimet uweseimet changed the title Use vector for INQUIRY data, made Inquiry() and other methods const Use vector for INQUIRY data, LUN list can have gaps, made methods const Mar 1, 2022

return size;
// Optical memory device, SCSI-2, removable
return PrimaryDevice::Inquiry(7, 2, true);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the SCSI Type (first argument) should be some sort of constant/enum/something to make this more readable. This PR has already been hanging out too long, so this would be a nice-to-have change somewhere down the line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akuker Yes, you are right. I will prepare a new ticket for that.

@akuker akuker merged commit 1df7cdb into develop Mar 2, 2022
@akuker akuker deleted the feature_inquiry_cleanup branch March 2, 2022 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SCSI spec misunderstanding: LUNs do *not* have to be consecutive All devices shall use PrimaryDevice::Inquiry()

2 participants