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

Add error handling code for some external APIs #17458

Merged
merged 1 commit into from Apr 18, 2020
Merged

Add error handling code for some external APIs #17458

merged 1 commit into from Apr 18, 2020

Conversation

ZhouyangJia
Copy link
Contributor

@ZhouyangJia ZhouyangJia commented Mar 9, 2020

This patch adds error handling for fopen and socket,
and fixes the issues mentioned in #13607.

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@ZhouyangJia ZhouyangJia marked this pull request as ready for review March 9, 2020 04:33
@ZhouyangJia ZhouyangJia requested a review from lrusak as a code owner March 9, 2020 04:33
xbmc/network/Socket.cpp Outdated Show resolved Hide resolved
@phunkyfish
Copy link
Contributor

@ZhouyangJia you should probably squash your last 2 commits as the reinterpret_cast really belongs in the second commit.

@phunkyfish
Copy link
Contributor

@ZhouyangJia you should only have 2 commits and no merge commits.

xbmc/platform/linux/storage/UDevProvider.cpp Outdated Show resolved Hide resolved
xbmc/platform/linux/storage/UDevProvider.cpp Outdated Show resolved Hide resolved
xbmc/platform/linux/storage/UDevProvider.cpp Outdated Show resolved Hide resolved
xbmc/platform/linux/storage/UDevProvider.cpp Outdated Show resolved Hide resolved
xbmc/platform/linux/storage/UDevProvider.cpp Outdated Show resolved Hide resolved
@phunkyfish phunkyfish added PR Cleanup: Merge Candidate Candidate for merging after PR cleanup Type: Fix non-breaking change which fixes an issue labels Mar 10, 2020
@phunkyfish
Copy link
Contributor

phunkyfish commented Mar 10, 2020

Please don't close and reopen a new PR, just fix the commits in this PR. If you need some guidance on how to do this happy to help.

Check the return value of socket and add a log message when socket fails.

Add error handling for fopen, or the program will crash at "while (fgets(buf, sizeof (buf), fp))" since fp is null.

Change NULL to nullptr in this file for consistency.
@phunkyfish
Copy link
Contributor

Ok, looking good. @AlwinEsch are you happy with this now?

@ZhouyangJia
Copy link
Contributor Author

I appreciate your patience and kind reply. As you can see, I'm struggling to figure out how the PR works. Not sure if I do the thing right this time.

Thanks

@phunkyfish
Copy link
Contributor

That's ok, feel free to ask questions if you not sure what to do ;)

Copy link
Member

@AlwinEsch AlwinEsch left a comment

Choose a reason for hiding this comment

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

I'm happy 😁

@AlwinEsch AlwinEsch added this to the Matrix 19.0-alpha 1 milestone Mar 10, 2020
Copy link
Contributor

@phunkyfish phunkyfish left a comment

Choose a reason for hiding this comment

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

Changes look good.

@phunkyfish
Copy link
Contributor

@ZhouyangJia once Jenkins is happy we can merge this.

@phunkyfish phunkyfish merged commit 0e954de into xbmc:master Apr 18, 2020
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 20, 2020
Add error handling code for some external APIs
@MilhouseVH
Copy link
Contributor

MilhouseVH commented Apr 21, 2020

This PR is responsible for the following crash on kodi startup (50/50 chance of hitting it):

Apr 21 16:11:31 nuc systemd[1]: Started Kodi Media Center.
Apr 21 16:11:31 nuc systemd[1]: systemd-update-utmp-runlevel.service: Succeeded.
Apr 21 16:11:31 nuc systemd[1]: Finished Update UTMP about System Runlevel Changes.
Apr 21 16:11:31 nuc kodi.sh[1865]: libva info: VA-API version 1.7.0
Apr 21 16:11:31 nuc kodi.sh[1865]: libva info: User environment variable requested driver 'iHD'
Apr 21 16:11:31 nuc kodi.sh[1865]: libva info: Trying to open /usr/lib/dri/iHD_drv_video.so
Apr 21 16:11:31 nuc kodi.sh[1865]: libva info: Found init function __vaDriverInit_1_7
Apr 21 16:11:31 nuc kodi.sh[1865]: libva info: va_openDriver() returns 0
Apr 21 16:11:33 nuc kodi.sh[1865]: Assertion 'close_nointr(fd) != -EBADF' failed at src/basic/fd-util.c:71, function safe_close(). Aborting.
Apr 21 16:11:33 nuc kodi.sh[1861]: Aborted (core dumped)

Crashlogs:

Ubuntu 19.10/x86_64: http://ix.io/2iYH
LibreELEC x86_64: http://ix.io/2iWY
LibreELEC RPi2: http://ix.io/2iZL

After reverting this PR the crashes stop.

@a1rwulf a1rwulf mentioned this pull request Apr 22, 2020
13 tasks
Maven85 pushed a commit to Maven85/kodi that referenced this pull request May 5, 2020
Add error handling code for some external APIs
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 3, 2020
Add error handling code for some external APIs
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
Add error handling code for some external APIs
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
Add error handling code for some external APIs
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
Add error handling code for some external APIs
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
Add error handling code for some external APIs
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
Add error handling code for some external APIs
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
Add error handling code for some external APIs
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
Add error handling code for some external APIs
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 7, 2020
Add error handling code for some external APIs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR Cleanup: Merge Candidate Candidate for merging after PR cleanup Type: Fix non-breaking change which fixes an issue v19 Matrix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants