Opened 5 years ago

Closed 5 years ago

#299 closed ожидается проверка (задача сдана)

WW_4

Reported by: Jura Khudyakov Owned by: Sokolov Viacheslav
Component: WW_mergesort Version: 2.0
Keywords: Cc:

Description


Change History (4)

comment:1 Changed 5 years ago by Jura Khudyakov

Попробовал также с помощью ulimit ограничить количество используемой памяти, чтобы попробовать имитировать memory limit. К сожалению, именно с его помощью ограничить память и сымитировать ситуацию на настоящих входных данных не удалось.
Зато попробовал malloc(1000000000000) (или что-то вроде), и ошибка обработалась корректно
(собирал в release, потому fsanitize допустим сам ловит эту ошибку, быстрее, чем я)
Не подскажете, как правильно пользоваться ulimit и как задавать ограничения программе на время и память исполнения?

comment:2 Changed 5 years ago by Sokolov Viacheslav

Type: ожидается проверкаожидаются исправления

ulimit задает на самом деле soft limit (это зависит от конкретной утилиты, конечно), подробно расписал на wiki.

Компараторы: потеряна константность CONST char * casted_a = *(CONST char**)a;
нет проверок на not null
возможно, отдельная переменная в данном случае не улучшает читаемость.

el_type же не нужен? лучше просто strcmp с argv, а так еще и переполнение буфера можно схлопотать ненароком

Обращаю внимание на английский в комментариях: это так называемый runnglish. В английском нет такого обилия запятых, языковые конструкции устроены иначе.

Да, 0 элементов нужно отдельно обработать, потому что результат вызова malloc(0) не специфирован
https://en.cppreference.com/w/c/memory/malloc

If size is zero, the behavior is implementation defined (null pointer may be returned, or some non-null pointer may be returned that may not be used to access storage, but has to be passed to free).

int не нужно парсить вручную, есть sscanf / atoi

Почему из main возвращается именно -1?

В случае сортировки строк аллокация на самом деле не нужна, можно сортировать argv

Проблема с копированием кода в main.c решается, например, так: заводится описание типа, которое включает в себя comparator, initializer, printer; по первой строке конструируется нужное описание типа, дальше код универсальный. Вся работа с конкретным типом располагается в одном месте (например, в отдельном файле). Переход из типизированного в нетипизированное происходит через type erasure, то есть через void*.

mergesort: хорошие проверки, к ним можно добавить еще: elements * element_size не переполняется; массив в конце действительно отсортирован

можно использовать shirt-circuit computation ​https://en.cppreference.com/w/c/language/operator_logical , чтобы сократить количество if ... return -1

В реализации mergesort на самом деле достаточно N дополнительной памяти, а не 2N (и одна аллокация).

Операцию "заполнение массива" может быть вынести отдельно в my_memcpy

В целом mergesort можно реализовать раза в два короче (по количеству содержательных строк, символов, ...).

comment:3 Changed 5 years ago by Jura Khudyakov

Type: ожидаются исправленияожидается проверка
Version: 1.02.0

comment:4 Changed 5 years ago by Sokolov Viacheslav

Resolution: задача сдана
Status: assignedclosed

В других компараторах тоже потеряна константность при преобразовании указателей.

56 array = (int *)malloc(el_count * el_size);
лишний cast (int *)

 64         for (size_t i = 0;i < el_count;++i)
 65         {
 66             casted_array[i] = atoi(argv[2+i]);
 67         }
 68         comp = comp_int;
 69 
 70         int return_code = mergesort(array,el_count,el_size,comp);
 71 

расстановка пробелов после ';' и ',' улучшит читаемость

 42     int is_int = strcmp(argv[1],"int");
 43     int is_char = strcmp(argv[1],"char");
 44     int is_str = strcmp(argv[1],"str");

лучше сразу сравнить с 0, чтобы название соответствовало содержимому переменной (сейчас можно подумать, что там хранится булево значение, то есть is_int = 0 = false => не int)

  8 char *buffer = NULL;
  9 char *casted_el_p1 = NULL, *casted_el_p2 = NULL;

Глобальные переменные - это плохо. В библиотечном коде это очень плохо.
Вызов функции меняет глобальное состояние; в многопоточной среде (если вызовы происходят из разных потоков) это просто не будет работать. Старайтесь избегать глобальных состояний.
Сходу не нашел другой статьи, но тезисно вроде бы здесь по делу, хотя меня и напрягает php и некоторые другие моменты https://habr.com/ru/company/mailru/blog/454946/

 42     if (buffer == NULL || casted_el_p1 == NULL || casted_el_p2 == NULL)
 43     {
 44         //according to C standard free(NULL) do nothing
 45         free(buffer);
 46         free(casted_el_p1);
 47         free(casted_el_p2);
 48         return -1;
 49     }

можно было бы использовать need_to_free

Доп. задание выполнено, балл засчитан

Note: See TracTickets for help on using tickets.