My code works when i run on my laptop but when i upload it to hackerrank I get error at strtol

Question

i am doing a problem from hackerrank here is the problem: https://www.hackerrank.com/challenges/time-conversion/problem

tl;dr we take time like 08:00:00PM and convert it to 20:00:00

the str_split function i take from this site: Split string with delimiters in C

first here is my full code:

#include <stdio.h>
#include <string.h>
#include <stdbool.h>
#include <stdlib.h>
#include <assert.h>

char** str_split(char* a_str, const char a_delim)
{
    char** result    = 0;
    size_t count     = 0;
    char* tmp        = a_str;
    char* last_comma = 0;
    char delim[2];
    delim[0] = a_delim;
    delim[1] = 0;

    /* Count how many elements will be extracted. */
    while (*tmp)
    {
        if (a_delim == *tmp)
        {
            count++;
            last_comma = tmp;
        }
        tmp++;
    }

    /* Add space for trailing token. */
    count += last_comma < (a_str + strlen(a_str) - 1);

    /* Add space for terminating null string so caller
       knows where the list of returned strings ends. */
    count++;

    result = malloc(sizeof(char*) * count);

    if (result)
    {
        size_t idx  = 0;
        char* token = strtok(a_str, delim);

        while (token)
        {
            assert(idx < count);
            *(result + idx++) = strdup(token);
            token = strtok(0, delim);
        }
        assert(idx == count - 1);
        *(result + idx) = 0;
    }

    return result;
}


char* timeConverse(char* chr)
{
    int del1 = 8;
    int hour = 0;
    char str[3];
    char *result = malloc(10 * sizeof(str));
    char** tokens;

    char s[11];
    for (int i = 0; i < 11; i++)
    {
        s[i] = *(chr + i);
    }

    bool time_of_day = 0; // 0 is morning, 1 is afternoon

    tokens = str_split(s, ':');
    char *endptr;
    hour = strtol(*(tokens), &endptr, 10); // THE MAIN PROBLEM FOR SEGFAULT I THINK?
    free(tokens);

    // check if it is morning or afternoon
    if (s[8] == 'P')
        time_of_day = 1;

    // if it is afternoon add 12 to hour
    if (time_of_day)
    {
        hour += 12;
        //
        // remove the hour from the timer
        memmove(&s[0], &s[0 + 1], strlen(s) - 0);
        memmove(&s[0], &s[0 + 1], strlen(s) - 0);

        // turn hour from int to string and store that to str
        sprintf(str, "%d", hour);
    }

    // remove the last 2 element from the list (PM or AM)
    s[strlen(s) - 1] = 0;
    s[strlen(s) - 1] = 0;

    // add hour to the min and second and store to result
    sprintf(result, "%s%s", str, s);

    // print out the result
    return result;
    free(result);
}

int main(void)
{
    // this is just a test
    char* time = "07:05:45PM";
    char* result = timeConverse(time);
    printf("%s\n", result);
    return 0;
}

and here is my code on hacker rank:

#include <assert.h>
#include <limits.h>
#include <math.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char* readline();

/*
 * Complete the timeConversion function below.
 */

/*
 * Please either make the string static or allocate on the heap. For example,
 * static char str[] = "hello world";
 * return str;
 *
 * OR
 *
 * char* str = "hello world";
 * return str;
 *
 */

char **str_split(char *a_str, const char a_delim) {
  char **result = 0;
  size_t count = 0;
  char *tmp = a_str;
  char *last_comma = 0;
  char delim[2];
  delim[0] = a_delim;
  delim[1] = 0;

  /* Count how many elements will be extracted. */
  while (*tmp) {
    if (a_delim == *tmp) {
      count++;
      last_comma = tmp;
    }
    tmp++;
  }

  /* Add space for trailing token. */
  count += last_comma < (a_str + strlen(a_str) - 1);

  /* Add space for terminating null string so caller
     knows where the list of returned strings ends. */
  count++;

  result = malloc(sizeof(char *) * count);

  if (result) {
    size_t idx = 0;
    char *token = strtok(a_str, delim);

    while (token) {
      assert(idx < count);
      *(result + idx++) = strdup(token);
      token = strtok(0, delim);
    }
    assert(idx == count - 1);
    *(result + idx) = 0;
  }

  return result;
}

char* timeConversion(char* chr) {
    /*
     * Write your code here.
     */
    int del1 = 8;
    int hour = 0;
    char str[3];
    char *result = malloc(10 * sizeof(str));
    char **tokens;

    char s[11];
    for (int i = 0; i < 11; i++) {
      s[i] = *(chr + i);
    }

    bool time_of_day = 0; // 0 is morning, 1 is afternoon

    tokens = str_split(s, ':');
    char *endptr;
    hour = strtol(*(tokens), &endptr, 10);
    free(tokens);

    // check if it is morning or afternoon
    if (s[8] == 'P')
      time_of_day = 1;

    // if it is afternoon add 12 to hour
    if (time_of_day) {
      hour += 12;
      //
      // remove the hour from the timer
      memmove(&s[0], &s[0 + 1], strlen(s) - 0);
      memmove(&s[0], &s[0 + 1], strlen(s) - 0);

      // turn hour from int to string and store that to str
      sprintf(str, "%d", hour);
    }

    // remove the last 2 element from the list (PM or AM)
    s[strlen(s) - 1] = 0;
    s[strlen(s) - 1] = 0;

    // add hour to the min and second and store to result
    sprintf(result, "%s%s", str, s);

    // print out the result
    return result;
    free(result);
}

int main()
{
    FILE* fptr = fopen(getenv("OUTPUT_PATH"), "w");

    char* s = readline();

    char* result = timeConversion(s);

    fprintf(fptr, "%s\n", result);

    fclose(fptr);

    return 0;
}

char* readline() {
    size_t alloc_length = 1024;
    size_t data_length = 0;
    char* data = malloc(alloc_length);

    while (true) {
        char* cursor = data + data_length;
        char* line = fgets(cursor, alloc_length - data_length, stdin);

        if (!line) { break; }

        data_length += strlen(cursor);

        if (data_length < alloc_length - 1 || data[data_length - 1] == '\n') { break; }

        size_t new_length = alloc_length << 1;
        data = realloc(data, new_length);

        if (!data) { break; }

        alloc_length = new_length;
    }

    if (data[data_length - 1] == '\n') {
        data[data_length - 1] = '\0';
    }

    data = realloc(data, data_length);

    return data;
}

it works on my pc but when i run my code on hackerrank, i upload the code into the timeConversion function, i got this error:

 GDB trace:
Reading symbols from solution...done.
[New LWP 11121]
Core was generated by `solution'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  __GI_____strtol_l_internal (
    nptr=0x5847d900 <error: Cannot access memory at address 0x5847d900>, 
    endptr=0x7ffd1f2ba350, base=10, group=<optimized out>, 
    loc=0x7f60be451560 <_nl_global_locale>) at ../stdlib/strtol_l.c:292
#0  __GI_____strtol_l_internal (
    nptr=0x5847d900 <error: Cannot access memory at address 0x5847d900>, 
    endptr=0x7ffd1f2ba350, base=10, group=<optimized out>, 
    loc=0x7f60be451560 <_nl_global_locale>) at ../stdlib/strtol_l.c:292
#1  0x0000555a56bd0d43 in timeConversion (chr=<optimized out>)
    at solution.c:89
#2  0x0000555a56bd0a4e in main () at solution.c:126

i assume that the hackerrank compiler different from my compiler computer. i compile the code using gcc (Debian 8.2.0-12) 8.2.0.

i don't want u guys to read a lot of code so i cut it, the function i used is the same as the answer function from this link: https://stackoverflow.com/questions/9210528/split-string-with-delimiters-in-c — haxerl, Dec 29 '18 at 16:11
The complier still needs to read `str_split()` definition to know how to properly call it. Look to posting a [MCVE] — chux - Reinstate Monica, Dec 29 '18 at 16:12
chux ask if the signature of `str_split` is known when it is called, in case the implicit declaration was not teh right one — bruno, Dec 29 '18 at 16:13
yeah i did add it to the hackerrank code and it run fine so that's not the source of the problem — haxerl, Dec 29 '18 at 16:13
It looks like `split_str` may not have returned a valid address for `tokens`, you are relying there on _someone else's code_, and it is a somewhat heavyweight solution in and case. Why not simply use `sscanf()` to both extract the token and convert to integer values in one step? — Clifford, Dec 29 '18 at 16:19
yeah i did a solution with sscanf() but this error bother me so much so i want to ask u guys. I have already solve the problem with sscanf() — haxerl, Dec 29 '18 at 16:21
`free( result ) ;` is an unreachable statement - allocating in one function and returning that allocation is a bad idea unless that function is clearly a memory allocation function such as `malloc()`. It requires the caller to _know_ the memory needs freeing. Here the memory is never free'ed. — Clifford, Dec 29 '18 at 16:29
it is useless to extract the hour both by _sscanf_ and by `str_split`, just do the _strtol_ on 's', see my answer please — bruno, Dec 29 '18 at 16:34
The `split_str()` code is ill-advised in any case - who knows how it attracted 146 upvotes?! You have copied the bad design pattern of "hidden" dynamic allocation in both `readline()` and `timeConversion()` and similarly failed to free those allocations - it is nasty leaky code! — Clifford, Dec 29 '18 at 16:52
@Clifford the `readline()` one it is a build-in function from hackerrank but i must admit that my `timeConversion()` is a mess when i first try to tackle this challenge — haxerl, Dec 29 '18 at 16:57
Perhaps that just confirms my low opinion of sites such as hackerrank if they trully encourage such poor practise. ` — Clifford, Dec 29 '18 at 17:00
@bruno - there is an even simpler trivial solution than either `sscanf()` or `strtol(). I think your suggestion to use `strtol` only does not avoid the need for the remainder of the function dealing rather clumsily with the PM suffix.You can use sscanf followed by sprintf and do the whole thing in two lines, but still not as simple as the direct character replacement in my answer. — Clifford, Dec 29 '18 at 17:07
@Clifford I just proposed a very little modification from his code, be sure my own version of _timeConversion_ would have been much simpler than his — bruno, Dec 29 '18 at 17:14
@bruno I don't doubt it - no criticism intended. It is a badly formed question IMO, and not clear whether he wants a solution or an explanation of the seg-fault. He has variously accepted both your and my answer, and I think neither offer an explanation. My answer addresses a fault in the so called "working" code before the failing code was posted when were being told they were the _same_! — Clifford, Dec 29 '18 at 17:18
@Clifford I will not learn you the real final goal is to have a working program, and the question asked is just to solve an intermediate problem on the way to the real goal. When a guy reads an overall solution he gets it, whatever it solves or not his local problem :-) — bruno, Dec 29 '18 at 17:41
@bruno : The question is _"this code works in one environment, but not this other - why?"_ rather _"how can I write code that does x"_. It is a bad question because originally we weer given one piece of code and told it was the same; then we were told that actually the code is different, and it turns out each one has errors (and different ones) in any case. You are right however; the error appears to be with the "someone-else's-code" he took on trust. — Clifford, Dec 29 '18 at 17:48

bruno · Answer 1 · 2018-12-29T16:31:46.140

2

your solution does not need str_split at all

just replace

tokens = str_split(s, ';');
char *endptr;
hour = strtol(*(tokens), &endptr, 10); // THE MAIN PROBLEM FOR SEGFAULT I THINK?
free(tokens);

by

char *endptr;
hour = strtol(s, &endptr, 10);

and of course remove char** tokens; and also int del1 = 8; being useless

edited Dec 29 '18 at 16:31

answered Dec 29 '18 at 16:24

bruno

32,421
7
25
37

@haxerl : While this answer may work and avoids the unnecessary `split_str()` function, and you have accepted it as the answer, in an earlier comment you stated that you already had an alternative solution, but that this error was bothering you. This answer does not reveal the cause of the error, it just another different solution. – Clifford Dec 29 '18 at 16:39

Clifford · Answer 2 · 2018-12-29T17:59:53.707

1

The problem is most likely this line in main():

char* time = "07:05:45PM";

Change to:

char time[] = "07:05:45PM";

In your solution time is a pointer to a literal constant, but split_str() modifies it.

The solution in general is over-engineered; the following will work:

char* timeConversion(char* s) 
{
    if (s[0] == '1' && s[1] == '2' ) 
    {
        if( s[8] == 'A' )
        {
            s[0] = '0' ;
            s[1] = '0' ;
        } 
    }
    else if( s[8] == 'P' ) 
    {
        s[0] += 1 ;
        s[1] += 2 ;
    }

    s[8] = 0 ;

    return s ;
}

edited Dec 29 '18 at 17:59

answered Dec 29 '18 at 16:24

Clifford

88,407
13
85
165

sorry but that work, the main reason why i put pointer so it look the same as hackerrank test case. The main code is the hackerrank one i just editted – haxerl Dec 29 '18 at 16:28
@haxerl you do not need at all to call `split_str` – bruno Dec 29 '18 at 16:30
@haxerl : You originally said the code worked on your PC (which I doubt) and that it was the same as that on hackerrank, which it is not. You are not helping yourself; I suggest you start again with a new question and delete this mess. I suggest that you deal only with the code that does not work, and post that code rather than posing code that works and asking about unseen code that does not! – Clifford Dec 29 '18 at 16:34
@haxerl ; The same criticism can be made of this answer that I made of bruno's. The first part is an issue with your "working" code not it turns out the code you are really asking about which is different., the second is an alternative solution not an answer to your question. I do not see it as any more acceptable as bruno's. It is just a badly formed question IMO. – Clifford Dec 29 '18 at 17:12
@Clifford i am pretty sure the hackerrank code work just as find on my pc it can even run on gdb online: https://onlinegdb.com/rJsVkNBbV but still give a segfault, but for a different reason – haxerl Dec 29 '18 at 17:30
@haxerl ; Yes the differences are probably related to the execution environment not the compiler - the code is wrong in any case, which makes it very difficult to answer your question since neither code truly works, and both are implausibly complicated in any event. – Clifford Dec 29 '18 at 18:02

My code works when i run on my laptop but when i upload it to hackerrank I get error at strtol

2 Answers2