Skip to content
Snippets Groups Projects

vlc_stream: cleanup error handling in vlc_stream_Read()

Open vlc_stream: cleanup error handling in vlc_stream_Read()
robUx4/vlc:stream-read into master
1 unresolved thread
Open Steve Lhomme requested to merge robUx4/vlc:stream-read into master
1 unresolved thread

These improvements make !2130 (merged) mostly useless.

Merge request reports

Members who can merge are allowed to add commits.

Merge request pipeline #236010 passed

Stage:

    Merge request pipeline passed for cbf059c6

    Approved by
    Thomas Guillem Jean-Baptiste Kempf
    Merge blocked: 2 checks failed
    Unresolved discussions must be resolved.
    Merge request must be rebased, because a fast-forward merge is not possible.

    Merge details

    • The source branch is 13898 commits behind the target branch.
    • 3 commits will be added to master.
    • Source branch will be deleted.

    Activity

    Filter activity
    • Approvals
    • Assignees & reviewers
    • Comments (from bots)
    • Comments (from users)
    • Commits & branches
    • Edits
    • Labels
    • Lock status
    • Mentions
    • Merge request status
    • Tracking
  • Thomas Guillem approved this merge request

    approved this merge request

  • Bart - The Review Bot added MRStatus::InReview label

    added MRStatus::InReview label

  • Steve Lhomme added 1 commit

    added 1 commit

    • 380c8297 - vlc_stream: don't read more than SSIZE_MAX at once in vlc_stream_Read()

    Compare with previous version

  • Steve Lhomme resolved all threads

    resolved all threads

  • Steve Lhomme added 1 commit

    added 1 commit

    • cbf059c6 - vlc_stream: don't read more than SSIZE_MAX at once in vlc_stream_Read()

    Compare with previous version

  • Romain Vimont
    Romain Vimont @rom1v started a thread on commit 99ad2e8d
    • src/input/stream.c
    501 501 {
    502 502 if (vlc_killed())
    503 503 break;
    504 continue;
    504 if (errno == EINTR || errno == EAGAIN)
    • Romain Vimont
      Romain Vimont @rom1v ·
      Developer

      IIUC, it's currently not in the contract of vlc_stream_ReadPartial() to set errno on error (and in practice, it is not always set).

    • Rémi Denis-Courmont
      Rémi Denis-Courmont @Courmisch ·
      Maintainer

      Yeah. For better or worse, the convention so far is to return 0 on fatal error. If the result is negative, then that is not a fatal error.

      Obviously this creates a problem that fatal errors cannot be distinguished between one another, or with normal end-of-stream, but that is the current state. As long as that remains true, this change is incorrect and may cause fatal errors.

    • Please register or sign in to reply
  • Jean-Baptiste Kempf approved this merge request

    approved this merge request

  • Bart - The Review Bot added MRStatus::Stale label and removed MRStatus::InReview label

    added MRStatus::Stale label and removed MRStatus::InReview label

  • Steve Lhomme mentioned in merge request !3800 (merged)

    mentioned in merge request !3800 (merged)

    • Please register or sign in to reply

    VideoLAN code repository instance