Skip to content

Segfault in rtsp.c when using WRITEDATA #1880

Closed
@cmeister2

Description

@cmeister2

After the fix to rtsp.c in a14f715, I've tried to run a test where I set WRITEDATA and WRITEFUNCTION and rtsp.c is still failing. I think the issue is as below:

  writeit = data->set.fwrite_rtp?data->set.fwrite_rtp:data->set.fwrite_func;

  if(!data->set.fwrite_rtp && !data->set.is_fwrite_set &&
     !data->set.rtp_out) {
    /* if no callback is set for either RTP or default, the default function
       fwrite() is utilized and that can't handle a NULL input */
    failf(data, "No destination to default data callback!");
    return CURLE_WRITE_ERROR;
  }

  wrote = writeit(ptr, 1, len, data->set.rtp_out);

It doesn't suffice to simply use data->set.rtp_out - I think the userdata pointer has to be set up by the if statement as well. I think if the fwrite_func is being used then the data->set.out pointer should be passed to the callback instead of data->set.rtp_out.

Activity

cmeister2

cmeister2 commented on Sep 11, 2017

@cmeister2
ContributorAuthor

The following code appears to fix it but I'm not 100% sure if that's correct. In particular I'm not really sure where the RTP output code is set up.

  curl_write_callback writeit;
  void *user_ptr;

  if(len == 0) {
    failf(data, "Cannot write a 0 size RTP packet.");
    return CURLE_WRITE_ERROR;
  }

  if(data->set.fwrite_rtp) {
    writeit = data->set.fwrite_rtp;
    user_ptr = data->set.rtp_out;
  }
  else
  {
    writeit = data->set.fwrite_func;
    user_ptr = data->set.out;
  }

  if(!data->set.fwrite_rtp && !data->set.is_fwrite_set &&
     user_ptr == NULL) {
    /* if no callback is set for either RTP or default, the default function
       fwrite() is utilized and that can't handle a NULL input */
    failf(data, "No destination to default data callback!");
    return CURLE_WRITE_ERROR;
  }

  wrote = writeit(ptr, 1, len, user_ptr);
bagder

bagder commented on Sep 12, 2017

@bagder
Member

I did my change like that since I was convinced that CURLOPT_INTERLEAVEDATA was documented to get passed to the WRITEFUNCTION if no INTERLEAVEFUNCTION was set, but now when I read the docs again with an extra eye on this I can see no hint or indication that it would actually work like this.

But still the code does make this decision, so the question is then who's right. The code or the (lack of) docs saying it should work like that.

I think I'm inclined to rule against the code here and in favor of the documentation. So yes, let's pass the data to the regular callback + writedata unless the INTERLEAVEFUNCTION is set and then use both. The code can be simplified a bit then since it won't send a NULL to fwrite() using the default setup, so the entire extra precaution I added can be removed again:

  curl_write_callback writeit;
  void *user_ptr;

  if(len == 0) {
    failf(data, "Cannot write a 0 size RTP packet.");
    return CURLE_WRITE_ERROR;
  }

  if(data->set.fwrite_rtp) {
    writeit = data->set.fwrite_rtp;
    user_ptr = data->set.rtp_out;
  }
  else
  {
    writeit = data->set.fwrite_func;
    user_ptr = data->set.out;
  }

  wrote = writeit(ptr, 1, len, user_ptr);

Can you perhaps amend this by also making some clarification in the documentation?

locked as resolved and limited conversation to collaborators on May 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @bagder@cmeister2

        Issue actions

          Segfault in rtsp.c when using WRITEDATA · Issue #1880 · curl/curl