HW #1

[tarasov.denis]

Не очень понятно какие файлы могут быть в тестах

[Sokolov Viacheslav]

╰─>$ make
mkdir obj
gcc src/bmp.c -c -o obj/bmp.o -Iinclude -std=c11 -fsanitize=address -Wall -Wextra -Werror
gcc src/main.c -c -o obj/main.o -Iinclude -std=c11 -fsanitize=address -Wall -Wextra -Werror
src/main.c: In function ‘ErrorProc?’:
src/main.c:31:7: error: this statement may fall through [-Werror=implicit-fallthrough=]

printf("Wrong params for the crop\n");

src/main.c:32:5: note: here

case E_STEGO:
^~

cc1: all warnings being treated as errors
Makefile:15: recipe for target 'obj/main.o' failed
make: * [obj/main.o] Error 1

[tarasov.denis]

У меня gcc этой ошибки почему-то не показывал

[tarasov.denis]

На старой версии Ubuntu gcc не выдавал некоторые ошибки и утечки памяти

[Sokolov Viacheslav]

С бонусом, круто!

Он правда не работает:

=================================================================
==31280==ERROR: AddressSanitizer: attempting double-free on 0x7f04b4ebf800 in thread T0:
    #0 0x7f04b3f667b8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7b8)
    #1 0x56224052cfe1 in main (/home/nicesap/HSE/svn/tarasov.denis/hw_01/hw_01+0x1fe1)
    #2 0x7f04b3ab8b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #3 0x56224052c409 in _start (/home/nicesap/HSE/svn/tarasov.denis/hw_01/hw_01+0x1409)

0x7f04b4ebf800 is located 0 bytes inside of 786432-byte region [0x7f04b4ebf800,0x7f04b4f7f800)
freed by thread T0 here:
    #0 0x7f04b3f667b8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7b8)
    #1 0x56224052cfae in main (/home/nicesap/HSE/svn/tarasov.denis/hw_01/hw_01+0x1fae)
    #2 0x7f04b3ab8b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

previously allocated by thread T0 here:
    #0 0x7f04b3f66b50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
    #1 0x56224052d434 in load_bmp (/home/nicesap/HSE/svn/tarasov.denis/hw_01/hw_01+0x2434)
    #2 0x56224052c75b in main (/home/nicesap/HSE/svn/tarasov.denis/hw_01/hw_01+0x175b)
    #3 0x7f04b3ab8b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

SUMMARY: AddressSanitizer: double-free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7b8) in __interceptor_free
==31280==ABORTING

Основной функционал правда тоже не работает:
нужно пересчитывать Size в заголовке, XPelsPerMeter, YPelsPerMeter, SizeImage?.

Сейчас вижу, что много кода дублируется, постарайтесь сделать так, чтобы дублирований было меньше. Возможно, будет удобно использовать short-circuit evaluations (операторы

, &&)

Рекомендую использовать memcpy для эффективного копириования памяти.

В целом хорошая первая попытка.

[Sokolov Viacheslav]

src/main.c: In function ‘ErrorProc’:
src/main.c:31:7: error: this statement may fall through [-Werror=implicit-fallthrough=]
       printf("Wrong params for the crop\n");
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/main.c:33:5: note: here
     case E_STEGO:
     ^~~~
cc1: all warnings being treated as errors
Makefile:15: recipe for target 'obj/main.o' failed
make: *** [obj/main.o] Error 1

[Sokolov Viacheslav]

pragma pack стоит отключать
​https://stackoverflow.com/questions/3318410/pragma-pack-effect

также стоит добавить assert-ы, что размер структуры именно такой, как ожидается

E_NULL_ARGUMENT лучше убрать, вернуть assert-ы. Assert проверяет ошибки программиста, пользователю незачем знать, как именно программист ошибся. Проверять нужно то, что зависит не от программиста - аллокации памяти, результаты системных вызовов, ввод пользователя, ...
E_NULL_ARGUMENT противоречит принципу "fail fast" - останавливать исполнение всей программы, как только где-то есть ошибка программиста.

49 int edge = (4 - (3 * Info->Width % 4));
1) стоит вынести в отдельную функцию
2) edge - это ребро, имеет другой смысл. Здесь речь про alignment.

165 int tw = Info->Width;
166 Info->Width = Info->Height;
167 Info->Height = tw;
стоит завернуть в scope ({}), чтобы ограничить область видимости tw

в crop, rotate удобно было бы использовать memcpy

В stego ошибки не очень хорошо обрабатываются (только один код возврата)

в stego лучше завести явные функции set_last_bit, reset_last_bit, а также, возможно, функцию получения позиции по x,y,color

Сейчас в stego есть ошибка, связанная с форматом BMP. Если бы было выполнено то, что я написал строчкой выше, ее было бы труднее допустить и проще исправить.

[Sokolov Viacheslav]

Стиль:
main выглядит тяжеловесно (куча if-else), так называемая "лапша"

stego:
не хватает not null проверок