LINUX.ORG.RU

C, realloc, проблемы


0

1

Пытаюсь написать простенькую программку, которая бы динамически бы выделяла массив + при необходимости перевыделяла его в большую сторону: считывает символы с stdin, сохраняет, сортирует, выводит. После нескольких иттераций вылетает с ошибкой, примерно такого вида:

*** glibc detected *** ./psort: free(): invalid next size (fast): 0x08935008 ***

gcc version 4.4.0 20090506 (Red Hat 4.4.0-4) (GCC), собираю gcc -Wall -Werror -pedantic -ansi -g -o psort psort.c

Ещё просьба покритиковать сам код на стиль. И ещё, кто подскажет (не пойму где копать). Как обрабатывать ошибки, если введено не int? scanf просто "виснет", если, скажем, ввести 3 4 aa45. Как правильно поступать?


/* A simple program, that take integers as it's input,
 * sorts them using qsort function and prints results
 */

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

#define MIN_BUF_SIZE 1    /* Minimal allocated buffer size */

void print_buf(int buf[], size_t size);
int cmpintp(const void *p1, const void *p2);

int main(void)
{
  int *buf;
  size_t cur_buf_size;    /* Current size of allocated buffer */
  size_t buf_pos;      /* Current position at buffer */

  (void) printf("Please, enter integer numbers, and press ENTER to continue:\n\n");
  if( (buf = (int*)malloc(MIN_BUF_SIZE * sizeof(int))) == NULL ) {
    fprintf(stderr,"Could not allocate memory for buffer, exiting...\n");
    return EXIT_FAILURE;
  };
  cur_buf_size = MIN_BUF_SIZE;
  buf_pos = 0;

  do {
    fscanf(stdin,"%d",&buf[buf_pos++]);
    if( buf_pos >= cur_buf_size) { /* Buffer is small, reallocate mem for it */
      cur_buf_size *= 2;
      if( (buf = (int*)realloc(buf,cur_buf_size)) == NULL) {
	fprintf(stderr,"Could not reallocate memory for buffer, exiting...\n");
	return EXIT_FAILURE;
      }
    }

  } while(!feof(stdin) && !ferror(stdin));

  print_buf(buf, buf_pos);
  qsort(buf, buf_pos, sizeof(int), cmpintp);
  printf("Sorted array : \n");
  print_buf(buf, buf_pos);


  free(buf);

  return EXIT_SUCCESS;
}

void print_buf(int buf[], size_t size)
{
  size_t i;
  putchar('\n');
  for(i=0;i<size-1;++i) {
    (void) printf("%d ",buf[i]);
  }
  putchar('\n');
}

int cmpintp(const void *p1, const void *p2)
{
  if( *((int*)p1) >= *((int*)p2)) {
    return 1;
  } else {
    return 0;
  }
}

tz00
() автор топика
Ответ на: комментарий от tz00

realloc(buf,cur_buf_size * sizeof(int)) - чуешь?

>И ещё, кто подскажет (не пойму где копать). Как обрабатывать ошибки, если введено не int? scanf просто "виснет", если, скажем, ввести 3 4 aa45. Как правильно поступать?


Читай посимвольно, проверяя на цифры.
Или в строку.

tzukko
()
Ответ на: комментарий от tzukko

Да, спасибо, действительно забыл на sizeof(int) умножить. Про строчку - интересный вариант, можно попробовать.

tz00
() автор топика
Ответ на: комментарий от tz00

Ты, уж если назвал переменную что-то_size, то и делай ее размером, а не количеством элементов. Или назови соответственно.
Это к вопросу критики кода.

tzukko
()

К вопросу критики кода: ставь пробелы много и последовательно. А то после 'if(' пробел есть (хотя зачем он там?), а после каста '(int*)' нету.

bvvv
()
Ответ на: комментарий от tz00

По поводу критики кода:
--------------------

1. Не надо приводить возвращаемый результат malloc и realloc. Эти функции возвращают указатель на void, который автоматически преобразовывается в любой тип. Если приводить насильно (в твоем случае к int *), то в случае ошибки (скажем не объявлен прототип malloc), то компилятор предупреждений не выдаст.

2. Вместо buf = (int*)malloc(MIN_BUF_SIZE * sizeof(int)) лучше писать buf = malloc(MIN_BUF_SIZE * sizeof(*buf)). Так если ты захочешь поменять тип указателя, скажем с int * на int64_t, то править нужно будет только в одном месте - в точке объявления указателя.

3. А вообще зачем тебе malloc? Используй сразу realloc. При первом вызове он будет работать, как malloc.

4. fscanf(stdin,"%d",&buf[buf_pos++]);- Как ты думаешь сколько в этой строке ошибок? Подумай на досуге.

5. void print_buf(int buf[], size_t size); - Ну тут вместо int buf[] лучше const int *buf, потому что наплевать что ты передаешь массив или  указатель. Внутри функции print_buf он все равно указатель. А вот константным его надо сделать на всяк пожарный.

6. Вместо if( *((int*)p1) >= *((int*)p2)) лучше if( *((const int *)p1) >= *((const int *)p2)) . Небрежность в коде это зло. Если ты не собираешься менять значение переменной, то делай ее const.

7. Инициализируй переменные.

=======================================================
Обязательно к прочтению: man realloc, man fscanf, http://c-faq.com
Инструменты, которые должны быть всегда под рукой: gdb, valgrind.

Valgrind нашел все твои ошибки сразу. Поэтому пользуйся им всегда.

=======================================================

Ниже мой вариант уже без ликов и всяких там финтефлюшек. Я бы конечно по другому написал. В частности, не использовал бы fscanf, а читал бы посимвольно, но для учебного примера сойдет и так:

/* A simple program, that take integers as it's input,
 * sorts them using qsort function and prints results
 */

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

#define MIN_BUF_SIZE 1    /* Minimal allocated buffer size */

void print_buf(const int *buf, size_t size);
int cmpintp(const void *p1, const void *p2);

int main(void)
{
    int *buf = NULL;
    size_t cur_buf_size = MIN_BUF_SIZE;    /* Current size of allocated buffer */
    size_t buf_pos = 0;      /* Current position at buffer */

    (void) printf("Please, enter integer numbers, and press ENTER to continue:\n\n");

    while(!feof(stdin)) {
        if(!buf_pos || buf_pos >= cur_buf_size) { /* Buffer is small, reallocate mem for it */
            cur_buf_size *= 2;
            if((buf = realloc(buf, cur_buf_size * sizeof(*buf))) == NULL) {
                fprintf(stderr,"Could not reallocate memory for buffer, exiting...\n");
                return EXIT_FAILURE;
            }
        }
        if (fscanf(stdin,"%d",&buf[buf_pos]) != EOF) {
            buf_pos++;
        } else if (ferror(stdin) && errno) {
            perror("fscanf");
            return EXIT_FAILURE;
        }
    }

    print_buf(buf, buf_pos);
    qsort(buf, buf_pos, sizeof(*buf), cmpintp);
    printf("Sorted array : \n");
    print_buf(buf, buf_pos);


    free(buf);

    return EXIT_SUCCESS;
}

void print_buf(const int *buf, size_t size)
{
    size_t i;

    putchar('\n');
    for(i = 0; i < size; ++i) printf("%d ", buf[i]);
    putchar('\n');
}

int cmpintp(const void *p1, const void *p2)
{
    if( *((const int *)p1) >= *((const int *)p2)) {
        return 1;
    } else {
        return 0;
    }
}

badgopher
()
Ответ на: комментарий от badgopher

1. (f|s)scanf надежней сравнивать с количеством элементов, которые хочешь получить, а не с EOF. На досуге подумай почему.

2. Иногда надо поддерживать совместимость с C++ компиляторами, тогда (int*) будет к месту.

3. sizeof(int) / sizeof(*buf) это скорее вопрос религии. Для сложных типов все равно желательно делать что-то типа alloc_my_type/free_my_type, для простых типов и так можно поправить.

Reset ★★★★★
()
Ответ на: комментарий от Reset

1. Я же сказал, что вариант учебный :-) Цель написать абсолютно правильную прогу я перед собой не ставил. Просто исправил основные огрехи и применил пару приемов, которые я хотел продемонстрировать.

2. Зачем? Чтобы компилить g++? Не проще сразу написать на плюсах? Вообще в первый раз слышу такую причину. Если надо юзать код в плюсах, то этот код обычно оформляется в виде отдельной сишной библиотеки.

3. Никакой тут религии нет. Разницы действительно между sizeof(int) и sizeof(*buf) нету, но у sizeof(*buf) преймущество, потому что не надо закладываться на тип.

badgopher
()
Ответ на: комментарий от Reset

3. Это не вопрос религии. Это уменьшает вероятность трудноуловимых ошибок. Равно как и такое:

char buf[BUFSIZ];
snprintf(buf, sizeof(buf), "format", ...);

Вместо sizeof(buf) тоже можно спокойно написать BUFSIZ, и работать оно будет правильно. До тех пор, пока buf ненароком не станет buf[BUFSIZ/2], скажем. А если buf в добавок не локальная переменная, объявленная сразу над snprintf'ом, а поле в какой-нибудь структуре, объявленной где-нибудь далеко -- у-у-у, какая веселуха начинается.

kemm
()
Ответ на: комментарий от ratatosk

Вычитание вместо сравнения? Фууууу! Риальные пацаны вычисляют, где больше положительных бит, сдвигами и "^","&".

LamerOk ★★★★★
()
Ответ на: комментарий от LamerOk

Спасибо всем за комментарии. Про вычитание -- не пойдет, т.к. очень большая возможность возниконовения переполнения (для средних int'ов).

tz00
() автор топика
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.