Getting "Abort trap 6" using memset()

Question

I am relatively new to C, so please bear with me if this is an obvious question. I've looked all over SO for an answer, and have not been able to figure this out.

I am writing a simple calculator -- it will take a calculation from the user ("1 + 3", for example, and return the result. To keep things simple, I am setting a length for the input buffer and forcing the user to stay within those bounds. If they input too many characters, I want to alert them they have gone over the limit, and reset the buffer so that they can re-input.

This functionality works fine when they stay under the limit. It also correctly gives them a message when they go over the limit. However, when they try to input a valid calculation after having put in an invalid one, I get abort trap: 6. I know this has something to do with how I am resetting the array and managing the memory of that buffer, but my C skills are not quite sharp enough to diagnose the problem on my own.

If anybody could please take a look, I'd really appreciate it! I've pasted my code below.

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

#define BUFFER_SIZE 50

static void ready_for_input()
{
  printf(">  ");
}

static char *as_string(char buffer[], int size)
{
  char *result = (char *)malloc((size + 1) * sizeof(char));
  if (!result)
  {
    fprintf(stderr, "calculator: allocation error");
    exit(EXIT_FAILURE);
  }

  for (int i = 0; i < size; i++)
  {
    result[i] = buffer[i];
  }

  // to make it a valid string 
  result[size] = '\0';
  return result;
}

static char *read_line()
{
  // put the input into a buffer
  char buffer[BUFFER_SIZE], c;
  int len = 0;

  while (true)
  {
    c = getchar();
    if (c == EOF || c == '\n')
    {
      // reset if input has exceeded buffer length
      if (len > BUFFER_SIZE)
      {
        printf("Calculations must be under 100 characters long.\n");
        memset(buffer, 0, sizeof(buffer));
        len = 0;
        ready_for_input();
      }
      else
      {
        return as_string(buffer, len);
      }
    }
    else
    {
      buffer[len++] = c;
    }
  }
}

static void start_calculator()
{
  ready_for_input();
  char *line = read_line();
  printf("input received : %s", line);
}

int main(int argc, char *argv[])
{
  start_calculator();
}

`if (len > BUFFER_SIZE)` -> `if (len >= BUFFER_SIZE)`. And you should move the check _above_ `if (c == EOF || c == '\n')` - you are just writing into `buffer[len++]` without checking that `len` is smaller then it's size. — KamilCuk, Jun 06 '21 at 17:55
pencil-and-paper exercise: walk thru your code with buffer size set to 1 and see how overflow is handled. — jdigital, Jun 06 '21 at 17:57
oh, right, seems obvious now -- great learning opportunity, thank you ! — nickhealy, Jun 06 '21 at 18:08
you need to declare c as type int, which is the type of EOF and the type returned by getchar. — stark, Jun 06 '21 at 18:26

UziGoozie · Accepted Answer · 2021-06-06T20:25:49.907

You don't prevent the buffer overflow, because you are checking for it too late. You should check whether the user is about to exceed the buffer's size, before the user hits enter.

The code below improves a bit the way a buffer overflow is checked:

static char *read_line()
{
  // put the input into a buffer
  char buffer[BUFFER_SIZE];
  int c; // getchar should be assigned to an int
  int len = 0;

  while (true)
  {
    c = getchar();

    if (len >= BUFFER_SIZE)
    {
      // drop everything until EOF or newline
      while (c != EOF && c != '\n')
        c = getchar();
      printf("Calculations must be under 100 characters long.\n");
      memset(buffer, 0, sizeof(buffer));
      len = 0;
      ready_for_input();
    }
    else if (c == EOF || c == '\n')
    {
      return as_string(buffer, len);
    }
    else
    {
      buffer[len++] = c;
    }
  }
}

Another thing to notice is that gethchar() should be assigned to an int variable instead of char since you are checking for EOF (more info about this)

Finally, you may want to check for better ways to read a line in c, such as fgets, dynamically allocate memory for your buffer and using realloc (or a combination of malloc and memmove) to double the size when a limit is reached, or using getline.

great answer, thank you. I hear you about the better ways to read a line -- I wanted to implement something from scratch for the sake of learning, but if I were to make something not for learning I would absolutely use one of those other methods you mentioned. thanks again! — nickhealy, Jun 06 '21 at 18:42

Getting "Abort trap 6" using memset()

1 Answers1