0

The program finds a char with the smallest ascii code in a string and outputs it. My problem is in message: Segmentation fault(core dumped). Why and Where does it occur? Thanks for attention. 

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

int main(void) {    

char* str = NULL;    
int* mincode = NULL;    
int* count = NULL;    
char* mincodeChar = NULL;


 str = (char *) malloc(50 * sizeof(char));
 mincode = (int *) malloc(1 * sizeof(int));
 count = (int *) malloc(1 * sizeof(int));

 if (NULL == str || NULL == mincode || NULL == count){
        printf("Alloc error");
        return EXIT_FAILURE;
    }

fgets(str, 50, stdin);

printf("your string: ");
puts(str);

*mincode = (int)(str[*count]);
*mincodeChar = *(str + *count);

 for (*count = 0; str[*count] != '\0'; (*count)++) {

    if( (int)str[*count] < (*mincode)) {
    (*mincode) = (int)str[*count];
    mincodeChar = (str + *count);
    printf("%c", *mincodeChar);
    }
}

printf("your character: ");
printf("%c", *mincodeChar);

free(str);
free(mincode);
free(count);

return EXIT_SUCCESS;
}
Sergei
  • 3
  • 4
  • 1
    Why are you dynamically allocating memory, when you have compile-time fixed sizes? Especially, why allocate memory for *one* `int`? Why not use a simple `int` variable? – Some programmer dude Nov 21 '16 at 15:05
  • 2
    1) at `*mincode = (int)(str[*count]);` : `*count` is uninitialized. – BLUEPIXY Nov 21 '16 at 15:07
  • Also please see [this discussion about casting the result of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – Some programmer dude Nov 21 '16 at 15:07
  • 1
    Why so much dynamic memory allocation, by the way? How is having `int *count` better than just plain `int count`? – StoryTeller - Unslander Monica Nov 21 '16 at 15:10
  • In addition to previous comments: a debugger would have told you exactly at which line the segfault occured. Learn to use a debugger, it will save you a lot of time in the future. – Jabberwocky Nov 21 '16 at 15:12
  • 1
    It's written in the task - not to use ordinary var's. So, that's why – Sergei Nov 21 '16 at 15:12
  • @Someprogrammerdude What do u mean "uninitialized" ? – Sergei Nov 21 '16 at 15:23
  • 2
    It wasn't me posting that comment, but you have to remember that `malloc` only *allocate* memory, it doesn't initialize it in any way. So when you use `*count` before the loop you are getting an *indeterminate* value which will probably be very wrong and you might go out of bounds of the memory allocated for `str`. – Some programmer dude Nov 21 '16 at 15:26
  • 1
    Also note that `str[*count]` and `*(str + *count)` are the same. – Some programmer dude Nov 21 '16 at 15:26
  • [fix like this](http://ideone.com/fDoU3j) – BLUEPIXY Nov 21 '16 at 16:34
  • Welcome to Stack Overflow. It would appear that you need to learn how to use a debugger to step line-by-line through your code, which will likely allow you to easily pinpoint the nature and location of the issue you're having. Using a debugger is, for all intents and purposes, required knowledge for any programmer. For more info, see [How to debug small programs](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/). – Random Davis Nov 21 '16 at 16:50

2 Answers2

4
char* mincodeChar = NULL;
....
*mincodeChar = *(str + *count);

You dereference a NULL pointer.

The lessons to take from this are:

  1. Always initialize you variables to a valid value as soon as possible.
  2. Check pointers before dereferencing.
StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
  • but there is mincodeChar = (str + *count); and then i dereference mincodeChar. why Doesn't it work anyway? Does it cause a problem? – Sergei Nov 21 '16 at 15:32
  • @Sergei, `*mincodeChar = *(str + *count);` **is not** the same as `mincodeChar = (str + *count);`. The first accesses pointed-to values, the second accesses the pointers. – StoryTeller - Unslander Monica Nov 21 '16 at 15:53
0

Didn't check but

*mincode = (int)(str[*count]);
*mincodeChar = *(str + *count);

count contains a value not initialized. So it can be 0 like, more probably, something like 492892911039... never understood the randomness of the not initialized variables. And in this case you are attempting to open the position at 492892911039.... SIGSEGV? if you want 0 or explicitely set it to 0 or rather then malloc() call calloc(). Then the same as the others... why you allocate a single variable? ... It can happen, but is for foreign constraints, an example is when you call some API functions that only accept a void *... but when is possible to avoid is better avoid. Not only for lazyness. Most implementations of malloc are list.So more you allocate more long becomes this list that at each new allocation must be scored. So a bit it slows down your malloc(). Not of course in this case... but ever better allocate few big block of memory rather than a lot small

jurhas
  • 613
  • 1
  • 4
  • 12