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

Ota/fix sudo issue #1487

Merged
merged 24 commits into from
Dec 14, 2020
Merged

Ota/fix sudo issue #1487

merged 24 commits into from
Dec 14, 2020

Conversation

pvyawaha
Copy link
Contributor

Issue #, if available:

Description of changes:

  • This changes fixes the way path is built by using cwd instead of pwd without which the application could not be run in sudo.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

Co-authored-by: Shubham Divekar <divekarshubham@gmail.com>
divekarshubham
divekarshubham previously approved these changes Dec 12, 2020
cobusve
cobusve previously approved these changes Dec 13, 2020
yanjos-dev
yanjos-dev previously approved these changes Dec 13, 2020
Copy link
Contributor

@gkwicker gkwicker left a comment

Choose a reason for hiding this comment

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

See comments – also this new code contains 3 copies of essentially identical code to construct a fully-qualified pathname. That should be factored into a static utility method and reused in this module. This will make the code less complex, more readable, and easier to test.

platform/posix/ota_pal/source/ota_pal_posix.c Outdated Show resolved Hide resolved
platform/posix/ota_pal/source/ota_pal_posix.c Outdated Show resolved Hide resolved
platform/posix/ota_pal/source/ota_pal_posix.c Outdated Show resolved Hide resolved
platform/posix/ota_pal/source/ota_pal_posix.c Outdated Show resolved Hide resolved
Copy link
Contributor

@gkwicker gkwicker left a comment

Choose a reason for hiding this comment

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

Pointer handling prior to the call to strncat() is still wrong - see comments and suggested alternative. Also, getFilePathFromCWD() needs to return some type of status so that callers know when an error has occurred.

platform/posix/ota_pal/source/ota_pal_posix.c Outdated Show resolved Hide resolved
platform/posix/ota_pal/source/ota_pal_posix.c Show resolved Hide resolved
Copy link
Contributor

@gkwicker gkwicker left a comment

Choose a reason for hiding this comment

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

You're still not doing anything in these error cases. The code as written will attempt to open invalid filenames. See comments.

platform/posix/ota_pal/source/ota_pal_posix.c Outdated Show resolved Hide resolved
platform/posix/ota_pal/source/ota_pal_posix.c Outdated Show resolved Hide resolved
platform/posix/ota_pal/source/ota_pal_posix.c Outdated Show resolved Hide resolved
@cobusve cobusve dismissed gkwicker’s stale review December 14, 2020 02:35

This has all been addressed now

@abhidixi11 abhidixi11 merged commit cd3c925 into main Dec 14, 2020
@pvyawaha pvyawaha deleted the ota/fix_sudo_issue branch December 14, 2020 04:38
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.

6 participants