Skip to content

Bugfix: MODE SELECT for format page is incorrect (issue #818)#899

Merged
uweseimet merged 2 commits intodevelopfrom
fix_issue_818
Oct 10, 2022
Merged

Bugfix: MODE SELECT for format page is incorrect (issue #818)#899
uweseimet merged 2 commits intodevelopfrom
fix_issue_818

Conversation

@uweseimet
Copy link
Copy Markdown
Contributor

@uweseimet uweseimet commented Oct 8, 2022 •
edited
Loading

Successfully tested with software that can change the sector size before formatting. Unit tests were added for the updated code.

After fixing the descriptor and offset handling rascsi behaves like a Fujitsu M2624S drive and a MODE SELECT/FORMAT sequence works as far as rascsi can support it.

[2022-10-08 13:07:25.373] [debug] ++++ CMD ++++ void ScsiController::Execute() Executing command $1A
[2022-10-08 13:07:25.373] [debug] bool Dispatcher<T>::Dispatch(T*, scsi_defs::scsi_command) [with T = ModePageDevice] Executing ModeSense6 ($1A)
[2022-10-08 13:07:30.382] [debug] ++++ CMD ++++ void ScsiController::Execute() Executing command $1A
[2022-10-08 13:07:30.382] [debug] bool Dispatcher<T>::Dispatch(T*, scsi_defs::scsi_command) [with T = ModePageDevice] Executing ModeSense6 ($1A)
[2022-10-08 13:07:30.384] [debug] ++++ CMD ++++ void ScsiController::Execute() Executing command $15
[2022-10-08 13:07:30.384] [debug] bool Dispatcher<T>::Dispatch(T*, scsi_defs::scsi_command) [with T = ModePageDevice] Executing ModeSelect6 ($15)
[2022-10-08 13:07:30.421] [debug] ++++ CMD ++++ void ScsiController::Execute() Executing command $04
[2022-10-08 13:07:30.421] [debug] bool Dispatcher<T>::Dispatch(T*, scsi_defs::scsi_command) [with T = Disk] Executing FormatUnit ($04)


2022-10-08 13:08:47.508] [debug] bool Dispatcher<T>::Dispatch(T*, scsi_defs::scsi_command) [with T = ModePageDevice] Executing ModeSense6 ($1A)
[2022-10-08 13:08:54.682] [debug] ++++ CMD ++++ void ScsiController::Execute() Executing command $1A
[2022-10-08 13:08:54.682] [debug] bool Dispatcher<T>::Dispatch(T*, scsi_defs::scsi_command) [with T = ModePageDevice] Executing ModeSense6 ($1A)
[2022-10-08 13:08:54.684] [debug] ++++ CMD ++++ void ScsiController::Execute() Executing command $15
[2022-10-08 13:08:54.684] [debug] bool Dispatcher<T>::Dispatch(T*, scsi_defs::scsi_command) [with T = ModePageDevice] Executing ModeSelect6 ($15)
[2022-10-08 13:08:54.684] [warning] In order to change the sector size use the -b option when launching rascsi
[2022-10-08 13:08:54.685] [debug] Error status: Sense Key $05, ASC $26

@uweseimet uweseimet linked an issue Oct 8, 2022 that may be closed by this pull request
MODE SELECT for format page (page 3) is not working #818
Closed
@uweseimet uweseimet marked this pull request as ready for review October 8, 2022 17:49
akuker
akuker approved these changes Oct 9, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@akuker akuker left a comment

Choose a reason for hiding this comment

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

Approved @uweseimet . I'll let you do the merging. Thanks!

@rdmark
Copy link
Copy Markdown
Member

rdmark commented Oct 10, 2022

This might not have been caused by the changes in this PR, but I'm getting this compiler error with gcc 10.2.1 / Bullseye / ARM (Pi Zero W).
Please let me know if you want an issue ticket with more context.

g++ -O3 -Wall -Werror -Wextra -DNDEBUG -Wno-psabi -std=c++17 -iquote . -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE -MD -MP  -DFMT_HEADER_ONLY -DCONNECT_TYPE_FULLSPEC -c ./devices/device_factory.cpp -o obj/fullspec/device_factory.o
./devices/device_factory.cpp: In member function ‘std::__cxx11::list<std::__cxx11::basic_string<char> > DeviceFactory::GetNetworkInterfaces() const’:
./devices/device_factory.cpp:223:17: error: ‘char* strncpy(char*, const char*, size_t)’ specified bound 16 equals destination size [-Werror=stringop-truncation]
  223 |          strncpy(ifr.ifr_name, tmp->ifa_name, IFNAMSIZ); //NOSONAR Using strncpy is safe here
      |          ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make: *** [Makefile:156: obj/fullspec/device_factory.o] Error 1

@uweseimet
Copy link
Copy Markdown
Contributor Author

uweseimet commented Oct 10, 2022

@akuker Looks as if your suggested change (which I liked better than the previous code) is causing this issue. I will change the code back to using strcpy as part of this PR. I hope that's fine for you.

@uweseimet uweseimet merged commit 4e4c5b2 into develop Oct 10, 2022
@uweseimet uweseimet deleted the fix_issue_818 branch October 10, 2022 06:16
@uweseimet
Copy link
Copy Markdown
Contributor Author

uweseimet commented Oct 10, 2022

@rdmark I fixed the compilation issue (which was depending on the compiler used) and double-checked that it compiles again on bullseye.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Oct 10, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

64.7% 64.7% Coverage
0.0% 0.0% Duplication

@uweseimet
Copy link
Copy Markdown
Contributor Author

uweseimet commented Oct 10, 2022

@akuker @rdmark I think I know now how to make the compiler happy while still using strncpy:

strncpy(ifr.ifr_name, tmp->ifa_name, IFNAMSIZ - 1)

Actually the compiler is right: If you copy exactly IFNAMSIZ bytes the terminating null byte for the string will be missing.
We can change the code accordingly, or we can keep it as it is. Both solutions are safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akuker akuker akuker approved these changes

@erichelgeson erichelgeson Awaiting requested review from erichelgeson

@rdmark rdmark Awaiting requested review from rdmark rdmark is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MODE SELECT for format page (page 3) is not working

3 participants

@uweseimet @rdmark @akuker