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

lightware_laser_serial: fix pointer for enabling serial mode #21747

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Jun 21, 2023

Solved Problem

When trying to fix #21737 (review) and compiling make beaglebone_blue locally with arm-linux-gnueabihf-gcc (Ubuntu 11.3.0-1ubuntu1~22.04.1) 11.3.0 I got a compile warning -> error:

PX4-Autopilot/src/drivers/distance_sensor/lightware_laser_serial/lightware_laser_serial.cpp: In member function ‘virtual void LightwareLaserSerial::Run()’:
PX4-Autopilot/src/drivers/distance_sensor/lightware_laser_serial/lightware_laser_serial.cpp:311:39: error: ‘ssize_t write(int, const void*, size_t)’ reading 5 bytes from a region of size 4 [-Werror=stringop-overread]
  311 |                         (void)!::write(_fd, &data, strlen(data));
      |                                ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated due to -Wfatal-errors.

Solution

const char *data = "www\r\n";
Defines a cstring of 6 bytes: {'w', 'w', 'w', '\r', '\n', '\0'}

type of data: char const*
type of &data: char const**

So when we call write(_fd, &data, strlen(data)); then strlen(data) == 5 (without the terminating null character) and we send the 4 byte memory address of data + some additional random byte to the distance sensor.

Correct is write(_fd, data, strlen(data)); where char const* gets cast to const void * and we pass the pointer to the content of data so the 5 bytes w, w, w, \r, \n get sent out via serial.

The fundamental problem here is write() not being typesafe.

Changelog Entry

For release notes:

Bugfix: lightware distance sensor serial initialization

Alternatives

const char data[] = "www\r\n";
write(_fd, data, strlen(data));

would be correct as well but creates slightly more overhead since it makes a copy of the cstring on the stack and passes the pointer of that to write().

Test coverage

const char *data = "www\r\n";
Defines a cstring of 6 bytes: 'w', 'w', 'w', '\r', '\n', '\0'

type of data: char const*
type of &data: char const**

So when we call
write(_fd, &data, strlen(data));
then strlen(data) == 5
and we send the 4 byte memory address of data
+ some additional random byte.

Correct is
write(_fd, data, strlen(data));
where char const* gets casted to const void * and we pass
the pointer to the content of data.

The fundamental problem here is write() not being typesafe.
@MaEtUgR MaEtUgR added Drivers 🔧 Sensors, Actuators, etc bugfix 🐞 labels Jun 21, 2023
@MaEtUgR MaEtUgR requested review from bkueng and dakejahl June 21, 2023 12:45
@MaEtUgR MaEtUgR self-assigned this Jun 21, 2023
@bkueng bkueng merged commit 18d89e4 into main Jun 21, 2023
79 of 85 checks passed
@bkueng bkueng deleted the maetugr/lightware-laser-write-pointer branch June 21, 2023 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix 🐞 Drivers 🔧 Sensors, Actuators, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants