0

I have the following piece of code. What the program does is ask the user for 2 strings. For the first one i tried out using a memmory allocated string with malloc() and used getc() in order to het the input from the user. For the second string i used an array of characters with specified size and scanf(). The issue i am having is that scanf gets the value that exceeded from the getc() used some lines of code before. How can i stop this behaviour? Also is enaString[ctr] = '\0'; and diaxwristiki[LINESIZE-1] = '\0' considered undefined behaviour? Or this is the right way of adding the null terminating character into your string?

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


int main(int argc, char* argv[])
{
    int LINESIZE = 15; //maximum length of array of characters
    int ctr = 0;
    char * enaString = NULL, xaraktiras, diaxwristiki[LINESIZE];

    enaString = (char*)malloc(sizeof(char) * LINESIZE + 1);

    if(enaString == NULL)//check if the memory has free blocks
        {
            printf("error to initillize memory");
            exit(1);
        }
    printf("Eisagete xaraktira mikous %d :\n", LINESIZE);
    do{
        xaraktiras = getc(stdin); 
        enaString[ctr] = xaraktiras;
        ctr++;
        if (ctr == LINESIZE -1)  
        {
            break;
        }

    }while(xaraktiras != '\n' );

    enaString[ctr] = '\0'; //is this considered undefined behaviour?
    enaString = NULL;
    free(enaString);

    printf("eisagete mia leksi diaxwrismou :\n");//ask the user for another word.fails cause it keeps the getc() value from before
    scanf(" %15s",diaxwristiki);
    diaxwristiki[LINESIZE-1] = '\0';//is this undefined behaviour?

    printf("timi diaxwrismou %s\n", diaxwristiki);




}
Korpel
  • 2,432
  • 20
  • 30
  • `ctr = 0`. Then `enaString[ctr] = xaraktiras` -> `enaString[0] = xaraktiras`. Then `ctr++` (ctr is now 1). Then `enaString[ctr + 1] = '\0'` -> `enaString[2] = '\0'`, so enaString[0] is valid (result of getc). enaString[2] is NUL. enaString[1] is ??????. Then you set enaString to NULL, leaking the malloced memory and then free(NULL); Clean up these issues first, leaking memory can cause wierd behavior elsewhere. – Tibrogargan Aug 14 '16 at 00:35
  • you are right i have acctually fixed that in my piece of code and `enaString[ctr + 1] = '\0'` is changed to `enaString[ctr] = '\0'` – Korpel Aug 14 '16 at 00:38
  • @Tibrogargan setting a pointer to `NULL` and then freeing the allocated space is bad? Should i first free the memory and then set the pointer to `NULL`? – Korpel Aug 14 '16 at 00:43
  • 1
    Very much so. `malloc` returns a memory address. You need to pass that same memory address to `free`. Changing the value of `enaString` loses the address. The result of `free(NULL)` is never going to be good. Free the memory. Setting enaString to NULL is pointless unless you test enaSting (i.e `if (NULL != enaString)`) – Tibrogargan Aug 14 '16 at 00:45
  • @Tibrogargan `free(NULL)` is standard compliant and perfectly valid, you can pass to `free()` whatever `malloc()`/`calloc()`/`realloc()` return and `NULL` is one of such values. Also, `NULL != enaString` is really ugly and not very natural, and there is no need for that when compilers can warn about that. – Iharob Al Asimi Aug 14 '16 at 00:46
  • @iharob "not bad" != "good". Also, assume compiler doesn't suck you do – Tibrogargan Aug 14 '16 at 00:47
  • @Tibrogargan Explain to me how the result is not going to be good? Nothing will happen, at all. – Iharob Al Asimi Aug 14 '16 at 00:48
  • @iharob The statement "nothing bad will happen" is not even close to "a desired result will happen" – Tibrogargan Aug 14 '16 at 00:49
  • If the first string is too long to fit in the array you allocate, the unread characters will remain in the input buffer, waiting to be read by the next input operation. You could put a loop after the `do { … } while (…)` loop to read up to and including the newline if the last character read was not a newline. Your code should take steps to handle early EOF, too. See also [`scanf()` leaves the newline character in the buffer](https://stackoverflow.com/questions/5240789/scanf-leaves-the-new-line-char-in-buffer) — though it is somewhat the converse of the current problem, – Jonathan Leffler Aug 14 '16 at 01:24
  • how can i "flush" that input buffer @JonathanLeffler? – Korpel Aug 14 '16 at 13:11
  • 1
    I said how to do it: `static inline void gobble_eol() { int c; while ((c = getchar()) != EOF && c != '\n') ; }` and call `gobble_eol();` when you need to get rid of any residual data on the current input line. – Jonathan Leffler Aug 14 '16 at 14:39
  • really thanks for your answer @JonathanLeffler ! I am gonna try to understand better what is inline but really appreciate your help – Korpel Aug 14 '16 at 20:12

1 Answers1

1

To compile the information hidden in the comments below the OP's post and my own 2 cents:

#include <stdio.h>
#include <stdlib.h>
// you don't use anything from string.h in your version
//#include <string.h>

// Put simple constants here, for the preprocessor to process
#define LINESIZE 15

// you don't use the arguments, no need to put them here
int main( /*int argc, char *argv[] */ )
{
  // such constants are better put into a preprocessor directive
  //int LINESIZE = 15; //maximum length of array of characters
  int ctr = 0;

  // Sorted into three lines (three different types) better to read

  // No need to initialize enaString to NULL for malloc/calloc
  // It is a good idea to do for realloc(), safes you
  // the initial malloc() but you do not use realloc() here
  char *enaString;
  // (f)getc() and scanf() return an int
  int xaraktiras, ret_scanf;
  char diaxwristiki[LINESIZE];

  // no casting of malloc() in C
  enaString = /*(char*) */ malloc(sizeof(char) * LINESIZE + 1);
  if (enaString == NULL)        //check if the memory has free blocks
  {                             
    // use stderr stream for error output
    // (sderr might not be available but worth a try)
    // UX-tip: use the same language for errors that you
    // use for user interaction elsewhere
    fprintf(stderr, "error to initillize memory");
    // Use the macros from stdlib.h, the return values
    // are OS dependent and might not be 0 and 1 respectively
    exit(EXIT_FAILURE);
  }
  // you ask for a word of a certain size or only for a word?
  // (My Greek is not very good and Google is of not much help here)
  printf("Eisagete xaraktira mikous %d :\n", LINESIZE);
  do {
    // please be aware the getc() is in most cases implemented
    // as a macro, use fgetc() if you are not sure if that is
    // a problem (it is not here) because macros might get evaluated
    // more than once
    xaraktiras = getc(stdin);
    // you need to check for EOF somewhere. Here would be a good place
    if (xaraktiras == EOF) {
      // try it by pressing CTRL+D instead of feeding characters to getc()
      fprintf(stderr, "EOF found in getc() loop\n");
      // EOF might also indicate an error, see the handling of scanf() below
      // We don't bother with it now, we just exit
      exit(EXIT_FAILURE);
    }
    // no need for a cast here
    enaString[ctr] = xaraktiras;
    // put it after the check, otherwise you have an undefined
    // character at enaString[ctr]
    // ctr++; 
    if (ctr == LINESIZE - 1) {
      // You offered LINESIZE, have allocated LINESIZE+1, but only
      // allow LINESIZE-1
      // The user might be disappointed
      break;
    }
    ctr++;
    // No casting needed, because the type of a char constant is int
    // (yes, that means that things like "char c='STOP'" once worked and you
    //  were able to look for 0x53544f50 in the memory dump. Some compilers might
    //  still allow for it but it is not recommended)
  } while (xaraktiras != '\n');

  // slurp the rest up if there were more characters given
  // (check for EOF ommitted here but should be added, of course)
  if(xaraktiras != '\n'){
     while ((xaraktiras = getc(stdin)) != '\n');
  }

  // you go up to LINESIZE-1 now, so, together with the replacement of ctr++, it is OK
  enaString[ctr + 1] = '\0';    //is this considered undefined behavior?

  // don't just dump the painfully gathered characters, print them at least.
  // That way you'll find out that you included the '\n', too, which
  // might or might not have been your intent
  printf("enaString = \"%s\"\n",enaString);

  // To free the memory free() needs to know where it is and
  // the pointer enaString points to that memory. If you set
  // enaString to NULL free() does not know which memory to free
  // (worse: free(NULL) is allowed) and the memory
  // is left alone, crying, and is unreachable until the program ends,
  // a so called "memory leak"

  // enaString = NULL;
  // free(enaString);
  free(enaString);
  // I don't know who told you so, but it is indeed a good idea to set
  // the pointer to the free'd memory to NULL. Won't do anything here
  // but might safe you from a lot of headaches in large programs
  enaString = NULL;

  printf("eisagete mia leksi diaxwrismou :\n"); //ask the user for another word.

  // the variable "diaxwristiki" can hold 15 characters , "%15s" allows for 16, because
  // scanf() includes `\0` (EOS, NUL, nul, or whatever the kids call it today), too!

  // scanf() returns the number of elements (not characters!) read or EOF.

// for strerror()
#include <string.h>
// for errno
#include <errno.h>
  // reset errno, just in case
  errno = 0;
  if ((ret_scanf = scanf(" %14s", diaxwristiki)) == EOF) {
    // It also returns EOF in case of an error, so check for it
    if (errno != 0) {
      fprintf(stderr, "error in scanf: %s\n", strerror(errno));
      exit(EXIT_FAILURE);
    }
    // try it by pressing CTRL+D instead of feeding characters to scanf()
    fprintf(stderr, "EOF triggered by scanf()\n");
    // diaxwristiki might contain rubbish at this point, clear it
    diaxwristiki[0] = '\0';
  }
  // no need for adding EOS, scanf() already added it
  // diaxwristiki[LINESIZE-1] = '\0';//is this undefined behaviour?
  printf("timi diaxwrismou %s\n", diaxwristiki);
  // it's "int main()", so return something.
  exit(EXIT_SUCCESS);
}
deamentiaemundi
  • 5,502
  • 2
  • 12
  • 20
  • although i really appreciate for helping me out and showing me some mistakes i made in my code which is really good for me since i want to get a better grasp of the C language my problem is not fixxed. Let me be more specific. let's say i want to input the following string : `kjasldjalsjdlaskjdlasjdl`. It will only keep the 15 letters meaning that `enaString = "kjasldjalsjdlas"`. Then the program proceeds to the second input. In the scanf instead of asking the user for another input it stores this one `kjdlasjdl` instead which are the letters remaining from the first input! how can i avoid it? – Korpel Aug 14 '16 at 13:07
  • And ask the user for input instead of just saving those characters from before? – Korpel Aug 14 '16 at 13:08
  • Aaargh! Yes, I just forgot it. Think, I can't ignore it anymore, I'm in a really desperate need of a looong holiday now. How's the weather in Greece? – deamentiaemundi Aug 14 '16 at 14:16
  • thanks mate worked as expected. Really thanks for your answer since it helped me out a lot! the weather here in Greece is great. Although myself i am not a fan of hot weather i suggest you visit us. It's gonna be the best time of your life and ofc the best vacations you ever had. The people and the island here are such a memorable experience you shouldnt miss. have a good one – Korpel Aug 14 '16 at 20:00