2

I'm probably missing something obvious, but my C is pretty rusty and I'm not having any luck making sense of this. I have a loop where I want to iterate over an array of uint64_t values coming from libdvdnav and then format the values and insert them into a string.

The header for libdvdnav defines the function I'm calling thusly:

uint32_t dvdnav_describe_title_chapters(dvdnav_t *self, int32_t title, uint64_t **times, uint64_t *duration);

Here's how I'm defining the variables used and executing the call (dvdnav and args[0] are defined and initialized elsewhere):

uint64_t *times;
uint64_t duration;
uint32_t times_array_len;

times_array_len = dvdnav_describe_title_chapters(dvdnav, atoi(args[0]), &times, &duration);

The code below seems to work, and compiles & runs w/o error, but of course only the first value in the array is inserted:

int i = 0;
uint64_t a_time = times[0];

while(i < times_array_len){

    char time_json[100];
    sprintf(time_json, "{\"chapter\":\"%d\",\"time\":\"%u\"},", i, a_time);
    strcat(payload, time_json);
    i++;
} 

If I modify this to select each value in the array it still compiles clean, but throws a segfault at runtime:

int i = 0;

while(i < times_array_len){

    char time_json[100];
    sprintf(time_json, "{\"chapter\":\"%d\",\"time\":\"%u\"},", i, times[i]);
    strcat(payload, time_json);
    i++;
} 

I thought maybe there was something in one of the array elements that was a problem (a too-large value, unexpected, NULL, etc.) but even if I replace the variable i with a known-reasonable element (say, 0) it still segfaults.

I'm sure there's countless other improvements to be made here (safer allocations, overflow protection, etc.) but the part I'm struggling to decipher is getting those values out of the array an into my formatted string.

9
  • "throws a segfault at runtime" -- where? Fire up a debugger. Commented Oct 29, 2014 at 15:19
  • 2
    That strcat() call looks scary ... Commented Oct 29, 2014 at 15:20
  • 1
    There are 2 possible problems. First you have some mistakes in calculating times_array_len, and times is smaller then it is. Second your payload too short for save all this data. Commented Oct 29, 2014 at 15:21
  • 1
    Looks like the segfault is on the strcat() line @BrianCain Commented Oct 29, 2014 at 15:37
  • Given the above I increased the size of the payload buffer and got it to work; seems like I had done that before but perhaps not enough. I do want to make sure that's not just masking other problems though so I'm going to poke at it a bit before considering it fixed :) Commented Oct 29, 2014 at 15:45

1 Answer 1

1

How is payload defined? If it is too short to contain the strings then you will get a crash. You can tackle this in several ways:

1) Since you now the number of json entries will be times_array_len you can allocate the string on heap using malloc with the size 100 * times_array_len - you will never exceed that (again, not sure if it is smart using a fixed length for the json buffer), then strcat should be safe. You can even do direct sprintf calls into the payload buffer dirrectly since you will now how far the offset is by keeping track of the return value of sprintf. Something like this:

#include <stdlib.h>
#include <stdio.h>

int main(int argc, char** argv)
{
    __int64 pTimes[] = { 1, 2, 3 ,4};
    size_t nTimeCount = sizeof(pTimes) / sizeof(pTimes[0]);
    size_t nPayloadOffset = 0;
    char* pPayload = (char*)malloc(100 * nTimeCount);
    if (pPayload)
    {
        for (size_t nTimeIndex = 0; nTimeIndex < nTimeCount; ++nTimeIndex)
        {
            nPayloadOffset += sprintf(&pPayload[nPayloadOffset], "{\"chapter\":\"%d\",\"time\":\"%u\"},", nTimeIndex, pTimes[nTimeIndex]);
        }
        printf("%s\n", pPayload);
        free(pPayload);
    }
    return EXIT_SUCCESS;
}

2) To avoid running over the 100 character length on a single entry you could be wise and allocate the pPlayoad with an initial size, then calculate the size of each entry and reallocate the pPayload if it becomes too short

3) Use C++ and std::stringstream if C++ is an option:

#include <sstream>
#include <iostream>

int main(int argc, char** argv)
{
    __int64 pTimes[] = { 1, 2, 3 ,4};
    size_t nTimeCount = sizeof(pTimes) / sizeof(pTimes[0]);
    size_t nPayloadOffset = 0;
    std::stringstream JsonStream;
    for (size_t nTimeIndex = 0; nTimeIndex < nTimeCount; ++nTimeIndex)
    {
        JsonStream << "{\"chapter\":\"" << nTimeIndex << "\",\"time\":\"" << pTimes[nTimeIndex] << "\"},";
    }
    std::cout << JsonStream.str() << std::endl;
    return EXIT_SUCCESS;
}
Sign up to request clarification or add additional context in comments.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.