Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Oct 10, 2022

Conversation

uweseimet
Copy link
Contributor

@uweseimet uweseimet commented Oct 8, 2022

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
@uweseimet uweseimet marked this pull request as ready for review October 8, 2022 17:49
Copy link
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
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
Contributor Author

@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
Contributor Author

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

@sonarcloud
Copy link

sonarcloud 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
Contributor Author

@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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