0

I am mostly concerned with the find and separation function. When I reused them in another program, I found that they don't always work and sometimes show segmentation fault. I tried every possible thing I could do, but nothing worked.

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

int find(char *argument)
{
    int count = 0;
    int i = 0;
    int state = 0;
    while (argument[i] != '\0')
    {
        if (argument[i] == ' ' || argument[i + 1] == '\0')
        {
            state = 0;
        }
        else if (state == 0)
        {

            state = 1;
            count++;
        }
        i++;
    }
    return count;
}

char **sepration(int args, char str[])
{
    int i = 0;
    char **argv = (char **)malloc(args);

    int k = 0;
    int len = strlen(str);
    while (i < len)
    {

        if (str[i] != ' ')
        {
            char *st = &str[i];
            argv[k] = st;
            k++;
            while (str[i] != ' ' && str[i] != '\0' && i < len)
            {
                i++;
            }
            str[i] = '\0';
        }
        i++;
    }

    return argv;
}

int main()
{
    char argument[100];
    scanf("%[^\n]s", &argument);
    //finds the no of words in the given argument
    int args = find(argument);
    char **argv = (char **)malloc(args + 1);
    //stores the word separately in **char pointer array
    argv = sepration(args, argument);
    //adds the arguments whichs are numbers
    add(args, argv);
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Sunil Bhagat
  • 99
  • 10
  • Also, you have a mem leak in `main()`. You argv = malloc(...);` but then you re-assign `argv` in the next line. – 001 Aug 30 '18 at 17:52
  • the posted code does not compile! When compiling, always enable the warnings, then fix the warnings. (for `gcc`, at a minimum use: `-Wall -Wextra -Wconversion -pedantic -std=gnu17` ) Note: other compilers have different options to obtain the same thing – user3629249 Aug 30 '18 at 18:01
  • where is the function: `add()`? it is not listed in the source code nor in any of the included header files – user3629249 Aug 30 '18 at 18:03
  • in C, when calling any of the heap allocation functions: `malloc` `calloc` `realloc` 1) the returned type is `void*` which can be assigned to any pointer. Casting just clutters the code, making it more difficult to understand, debug, etc.. 2) always check (!=NULL) the returned value to assure the operation was successful – user3629249 Aug 30 '18 at 18:09
  • Check this [link](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) for why you should not cast the result of malloc – dodobhoot Aug 30 '18 at 18:10
  • regarding: `scanf("%[^\n]s", &argument);` The input format specifier used in this statement will stop inputting when it encounters a '\n' so the next character in `stdin` will NEVER be `s`. Suggest removing the trailing `s` – user3629249 Aug 30 '18 at 18:12
  • 1
    `char **argv = (char **)malloc(args);` Doesn't allocate the correct amount of mem. Should be: `char **argv = malloc(args * sizeof(char *));` – – 001 Aug 30 '18 at 18:16
  • when calling any of the `scanf()` family of functions, 1) always check the returned value (not the parameter values) to assure the operation was successful. 2) when using the input format specifiers '%s' and/or '%[...]', always include a MAX characters modifier that is 1 less than the length of the input buffer. Because those specifiers always append a NUL byte to the input. This avoids any possibility of a buffer overflow (and the resulting undefined behavior) – user3629249 Aug 30 '18 at 18:17
  • the posted code fails to pass the allocated memory to `free()` before exiting. The OS (probably) will clean up this mess when the program exits However, there are LOTS of reasons to not depend on the OS to cleanup sloppy code. – user3629249 Aug 30 '18 at 18:21
  • you might want to look at the function: `strtok()` for breaking the string into words. There is no guarantee that the string, entered by the user, does not contain alphabetic characters nor other punctuation marks besides ' '. Suggest performing a positive check for a digit rather than a negative check for a space – user3629249 Aug 30 '18 at 18:28
  • regarding: `while (str[i] != ' ' && str[i] != '\0' && i < len)` the last two expressions are both (effectively) checking for the same thing. You could simplify the code by eliminating one of those expressions – user3629249 Aug 30 '18 at 18:35

1 Answers1

0

The problem with your solution is that you are not allocating the memory properly.
Your double pointer argv should work as an array of strings. Hence argv should have enough memory allocated for the same. Then next question is how to allocate sufficient memory for the same. As argv should hold argc number of strings hence proper memory allocation will be
char **argv = malloc(args * sizeof(char *)); or,
char *argv[argc];

Modified code will look like 

char **sepration(int args, char str[])
{
    int i = 0;
    char **argv = malloc(args * sizeof(char *));

    int k = 0;
    int len = strlen(str);
    while (i < len)
    {
        if (str[i] != ' ')
        {
            char *st = &str[i];
            argv[k] = st;
            k++;
            while (str[i] != ' ' && str[i] != '\0' && i < len)
            {
                i++;
            }
            str[i] = '\0';
        }
        i++;
    }
    return argv;
}

int main()
{
    char argument[100];
    scanf("%[^\n]", argument);
    //finds the no of words in the given argument
    int args = find(argument);
    char **argv = malloc(args * sizeof(char *));
    //stores the word separately in **char pointer array
    argv = sepration(args, argument);
    for(int i = 0; i < args; i++)
        printf("%s\n", argv[i]);
}

Instead of char **argv = malloc(args * sizeof(char *)); you can use char *argv[argc]; as well. This will also give the same result.

dodobhoot
  • 493
  • 6
  • 15