HW #1

[Igor Engel]

Это в теории рабочая попытка на допбаллы, на практике вероятно надо ещё дебажить и искать утечки...

[Sokolov Viacheslav]

╰─>$ make
mkdir obj
gcc -c -Iinclude -Wall -Wextra -Werror -Wno-unused-variable -fsanitize=address -fsanitize=undefined -g3 src/main.c -o obj/main.o
gcc -c -Iinclude -Wall -Wextra -Werror -Wno-unused-variable -fsanitize=address -fsanitize=undefined -g3 src/bmp.c -o obj/bmp.o
In file included from src/bmp.c:3:0:
src/bmp.c: In function ‘get_pixel’:
src/bmp.c:114:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= x && x <= bmp->meta->image_width);
              ^
src/bmp.c:114:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= x && x <= bmp->meta->image_width);
              ^
src/bmp.c:115:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= y && y <= bmp->meta->height);
              ^
src/bmp.c:115:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= y && y <= bmp->meta->height);
              ^
src/bmp.c: In function ‘get_pixel_raw’:
src/bmp.c:124:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= x && x <= bmp->meta->image_width);
              ^
src/bmp.c:124:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= x && x <= bmp->meta->image_width);
              ^
src/bmp.c:125:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= y && y <= bmp->meta->height);
              ^
src/bmp.c:125:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= y && y <= bmp->meta->height);
              ^
src/bmp.c: In function ‘crop_image’:
src/bmp.c:165:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= x && x < x + w && x + w <= bmp->meta->image_width);
              ^
src/bmp.c:165:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= x && x < x + w && x + w <= bmp->meta->image_width);
              ^
src/bmp.c:166:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= y && y < y + h && y + h <= bmp->meta->height);
              ^
src/bmp.c:166:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
     assert(0 <= y && y < y + h && y + h <= bmp->meta->height);
              ^
cc1: all warnings being treated as errors
Makefile:17: recipe for target 'obj/bmp.o' failed
make: *** [obj/bmp.o] Error 1

[Igor Engel]

Поправил, но что интересно, у меня не ругался...

[Igor Engel]

Поменял все ассерты на проброс кодов возврата

[Sokolov Viacheslav]

src/stego.c: In function ‘map_char’:
src/stego.c:7:12: error: suggest parentheses around comparison in operand of ‘&’ [-Werror=parentheses]

if('A' <= a & a <= 'Z') {

^~

cc1: all warnings being treated as errors

наверное, имелось в виду &&

[Igor Engel]

Да, поправил.

Как заставить компилятор сильнее орать? А то все флаги включены, а он молчит...

[Sokolov Viacheslav]

От версии компилятора зависит. У меня gcc (Ubuntu 7.4.0-9ubuntu1~18.04.york0) 7.4.0
Точнее в конце.
Мой компилятор мог быть собран немного с другими опциями. Взят вот отсюда ​https://launchpad.net/~jonathonf (там вообще много полезного)

gcc -dumpspecs

*asm:
%{m16|m32:--32}  %{m16|m32|mx32:;:--64}  %{mx32:--x32}  %{msse2avx:%{!mavx:-msse2avx}}

*asm_debug:
%{%:debug-level-gt(0):%{gstabs*:--gstabs}%{!gstabs*:%{g*:--gdwarf2}}} %{fdebug-prefix-map=*:--debug-prefix-map %*}

*asm_final:
%{gsplit-dwarf: 
       objcopy --extract-dwo 	 %{c:%{o*:%*}%{!o*:%b%O}}%{!c:%U%O} 	 %{c:%{o*:%:replace-extension(%{o*:%*} .dwo)}%{!o*:%b.dwo}}%{!c:%b.dwo} 
       objcopy --strip-dwo 	 %{c:%{o*:%*}%{!o*:%b%O}}%{!c:%U%O}     }

*asm_options:
%{-target-help:%:print-asm-header()} %{v} %{w:-W} %{I*}  %{gz|gz=zlib:--compress-debug-sections=zlib} %{gz=none:--compress-debug-sections=none} %{gz=zlib-gnu:--compress-debug-sections=zlib-gnu} %a %Y %{c:%W{o*}%{!o*:-o %w%b%O}}%{!c:-o %d%w%u%O}

*invoke_as:
%{!fwpa*:   %{fcompare-debug=*|fdump-final-insns=*:%:compare-debug-dump-opt()}   %{!S:-o %|.s |
 as %(asm_options) %m.s %A }  }

*cpp:
%{posix:-D_POSIX_SOURCE} %{pthread:-D_REENTRANT}

*cpp_options:
%(cpp_unique_options) %1 %{m*} %{std*&ansi&trigraphs} %{W*&pedantic*} %{w} %{f*} %{g*:%{%:debug-level-gt(0):%{g*} %{!fno-working-directory:-fworking-directory}}} %{O*} %{undef} %{save-temps*:-fpch-preprocess} %(ssp_default)

*cpp_debug_options:
%{d*}

*cpp_unique_options:
%{!Q:-quiet} %{nostdinc*} %{C} %{CC} %{v} %{I*&F*} %{P} %I %{MD:-MD %{!o:%b.d}%{o*:%.d%*}} %{MMD:-MMD %{!o:%b.d}%{o*:%.d%*}} %{M} %{MM} %{MF*} %{MG} %{MP} %{MQ*} %{MT*} %{!E:%{!M:%{!MM:%{!MT:%{!MQ:%{MD|MMD:%{o*:-MQ %*}}}}}}} %{remap} %{g3|ggdb3|gstabs3|gcoff3|gxcoff3|gvms3:-dD} %{!iplugindir*:%{fplugin*:%:find-plugindir()}} %{H} %C %{D*&U*&A*} %{i*} %Z %i %{E|M|MM:%W{o*}}

*trad_capable_cpp:
cc1 -E %{traditional|traditional-cpp:-traditional-cpp}

*cc1:
%{!mandroid|tno-android-cc:%(cc1_cpu) %{profile:-p};:%(cc1_cpu) %{profile:-p} %{!mglibc:%{!muclibc:%{!mbionic: -mbionic}}} %{!fno-pic:%{!fno-PIC:%{!fpic:%{!fPIC: -fPIC}}}}}

*cc1_options:
%{pg:%{fomit-frame-pointer:%e-pg and -fomit-frame-pointer are incompatible}} %{!iplugindir*:%{fplugin*:%:find-plugindir()}} %1 %{!Q:-quiet} %{!dumpbase:-dumpbase %B} %{d*} %{m*} %{aux-info*} %{fcompare-debug-second:%:compare-debug-auxbase-opt(%b)}  %{!fcompare-debug-second:%{c|S:%{o*:-auxbase-strip %*}%{!o*:-auxbase %b}}}%{!c:%{!S:-auxbase %b}}  %{g*} %{O*} %{W*&pedantic*} %{w} %{std*&ansi&trigraphs} %{v:-version} %{pg:-p} %{p} %{f*} %{undef} %{Qn:-fno-ident} %{Qy:} %{-help:--help} %{-target-help:--target-help} %{-version:--version} %{-help=*:--help=%*} %{!fsyntax-only:%{S:%W{o*}%{!o*:-o %b.s}}} %{fsyntax-only:-o %j} %{-param*} %{coverage:-fprofile-arcs -ftest-coverage} %{fprofile-arcs|fprofile-generate*|coverage:   %{!fprofile-update=single:     %{pthread:-fprofile-update=prefer-atomic}}}

*cc1plus:


*link_gcc_c_sequence:
%{static:--start-group} %G %L %{static:--end-group}%{!static:%G}

*link_ssp:
%{fstack-protector|fstack-protector-all|fstack-protector-strong|fstack-protector-explicit:}

*ssp_default:
%{!fno-stack-protector:%{!fstack-protector-all:%{!ffreestanding:%{!nostdlib:%{!fstack-protector:-fstack-protector-strong}}}}} %{!Wformat:%{!Wformat=2:%{!Wformat=0:%{!Wall:-Wformat} %{!Wno-format-security:-Wformat-security}}}}

*endfile:
%{!mandroid|tno-android-ld:%{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s}    %{mpc32:crtprec32.o%s}    %{mpc64:crtprec64.o%s}    %{mpc80:crtprec80.o%s} %{fvtable-verify=none:%s;      fvtable-verify=preinit:vtv_end_preinit.o%s;      fvtable-verify=std:vtv_end.o%s}    %{static:crtend.o%s;      shared|!no-pie:crtendS.o%s;      :crtend.o%s}    crtn.o%s    %{fopenacc|fopenmp:crtoffloadend%O%s};:%{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s}    %{mpc32:crtprec32.o%s}    %{mpc64:crtprec64.o%s}    %{mpc80:crtprec80.o%s} %{shared: crtend_so%O%s;: crtend_android%O%s}}

*link:
%{!r:--build-id} %{!static:--eh-frame-hdr} %{!mandroid|tno-android-ld:%{m16|m32|mx32:;:-m elf_x86_64}                    %{m16|m32:-m elf_i386}                    %{mx32:-m elf32_x86_64}   --hash-style=gnu   --as-needed   %{shared:-shared}   %{!shared:     %{!static:       %{rdynamic:-export-dynamic}       %{m16|m32:-dynamic-linker %{muclibc:/lib/ld-uClibc.so.0;:%{mbionic:/system/bin/linker;:%{mmusl:/lib/ld-musl-i386.so.1;:/lib/ld-linux.so.2}}}}       %{m16|m32|mx32:;:-dynamic-linker %{muclibc:/lib/ld64-uClibc.so.0;:%{mbionic:/system/bin/linker64;:%{mmusl:/lib/ld-musl-x86_64.so.1;:/lib64/ld-linux-x86-64.so.2}}}}       %{mx32:-dynamic-linker %{muclibc:/lib/ldx32-uClibc.so.0;:%{mbionic:/system/bin/linkerx32;:%{mmusl:/lib/ld-musl-x32.so.1;:/libx32/ld-linux-x32.so.2}}}}}     %{static:-static}};:%{m16|m32|mx32:;:-m elf_x86_64}                    %{m16|m32:-m elf_i386}                    %{mx32:-m elf32_x86_64}   --hash-style=gnu   --as-needed   %{shared:-shared}   %{!shared:     %{!static:       %{rdynamic:-export-dynamic}       %{m16|m32:-dynamic-linker %{muclibc:/lib/ld-uClibc.so.0;:%{mbionic:/system/bin/linker;:%{mmusl:/lib/ld-musl-i386.so.1;:/lib/ld-linux.so.2}}}}       %{m16|m32|mx32:;:-dynamic-linker %{muclibc:/lib/ld64-uClibc.so.0;:%{mbionic:/system/bin/linker64;:%{mmusl:/lib/ld-musl-x86_64.so.1;:/lib64/ld-linux-x86-64.so.2}}}}       %{mx32:-dynamic-linker %{muclibc:/lib/ldx32-uClibc.so.0;:%{mbionic:/system/bin/linkerx32;:%{mmusl:/lib/ld-musl-x32.so.1;:/libx32/ld-linux-x32.so.2}}}}}     %{static:-static}} %{shared: -Bsymbolic}}

*lib:
%{!mandroid|tno-android-ld:%{pthread:-lpthread} %{shared:-lc}    %{!shared:%{profile:-lc_p}%{!profile:-lc}};:%{shared:-lc}    %{!shared:%{profile:-lc_p}%{!profile:-lc}} %{!static: -ldl}}

*link_gomp:


*libgcc:
%{static|static-libgcc:-lgcc -lgcc_eh}%{!static:%{!static-libgcc:%{!shared-libgcc:-lgcc --push-state --as-needed -lgcc_s --pop-state}%{shared-libgcc:-lgcc_s%{!shared: -lgcc}}}}

*startfile:
%{!mandroid|tno-android-ld:%{shared:;      pg|p|profile:gcrt1.o%s;      static:crt1.o%s;      !no-pie:Scrt1.o%s;      :crt1.o%s}    crti.o%s    %{static:crtbeginT.o%s;      shared|!no-pie:crtbeginS.o%s;      :crtbegin.o%s}    %{fvtable-verify=none:%s;      fvtable-verify=preinit:vtv_start_preinit.o%s;      fvtable-verify=std:vtv_start.o%s}    %{fopenacc|fopenmp:crtoffloadbegin%O%s};:%{shared: crtbegin_so%O%s;:  %{static: crtbegin_static%O%s;: crtbegin_dynamic%O%s}}}

*cross_compile:
0

*version:
7.4.0

*multilib:
. !m32 !m64 !mx32;32:../lib32:i386-linux-gnu m32 !m64 !mx32;64:../lib:x86_64-linux-gnu !m32 m64 !mx32;x32:../libx32:x86_64-linux-gnux32 !m32 !m64 mx32;

*multilib_defaults:
m64

*multilib_extra:


*multilib_matches:
m32 m32;m64 m64;mx32 mx32;

*multilib_exclusions:


*multilib_options:
m32/m64/mx32

*multilib_reuse:


*linker:
collect2

*linker_plugin_file:


*lto_wrapper:


*lto_gcc:


*post_link:


*link_libgcc:
%D

*md_exec_prefix:


*md_startfile_prefix:


*md_startfile_prefix_1:


*startfile_prefix_spec:


*sysroot_spec:
--sysroot=%R

*sysroot_suffix_spec:


*sysroot_hdrs_suffix_spec:


*self_spec:


*cc1_cpu:
%{march=native:%>march=native %:local_cpu_detect(arch)   %{!mtune=*:%>mtune=native %:local_cpu_detect(tune)}} %{mtune=native:%>mtune=native %:local_cpu_detect(tune)}

*link_command:
%{!fsyntax-only:%{!c:%{!M:%{!MM:%{!E:%{!S:    %(linker) %{!fno-use-linker-plugin:%{!fno-lto:     -plugin %(linker_plugin_file)     -plugin-opt=%(lto_wrapper)     -plugin-opt=-fresolution=%u.res     %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}}     }}%{flto|flto=*:%<fcompare-debug*}     %{flto} %{fno-lto} %{flto=*} %l %{static|shared|r:;!no-pie:-pie -z now} %{fuse-ld=*:-fuse-ld=%*}  %{gz|gz=zlib:--compress-debug-sections=zlib} %{gz=none:--compress-debug-sections=none} %{gz=zlib-gnu:--compress-debug-sections=zlib-gnu} -z relro %X %{o*} %{e*} %{N} %{n} %{r}    %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!nostartfiles:%S}}     %{static|no-pie:} %{L*} %(mfwrap) %(link_libgcc) %{fvtable-verify=none:} %{fvtable-verify=std:   %e-fvtable-verify=std is not supported in this configuration} %{fvtable-verify=preinit:   %e-fvtable-verify=preinit is not supported in this configuration} %{!nostdlib:%{!nodefaultlibs:%{%:sanitize(address):%{!shared:libasan_preinit%O%s} %{static-libasan:%{!shared:-Bstatic --whole-archive -lasan --no-whole-archive -Bdynamic}}%{!static-libasan:%{!fuse-ld=gold:--push-state} --no-as-needed -lasan %{fuse-ld=gold:--as-needed;:--pop-state}}}     %{%:sanitize(thread):%{static-libtsan:%{!shared:-Bstatic --whole-archive -ltsan --no-whole-archive -Bdynamic}}%{!static-libtsan:%{!fuse-ld=gold:--push-state} --no-as-needed -ltsan %{fuse-ld=gold:--as-needed;:--pop-state}}}     %{%:sanitize(leak):%{static-liblsan:%{!shared:-Bstatic --whole-archive -llsan --no-whole-archive -Bdynamic}}%{!static-liblsan:%{!fuse-ld=gold:--push-state} --no-as-needed -llsan %{fuse-ld=gold:--as-needed;:--pop-state}}}}} %o %{!nostdlib:%{!nodefaultlibs:%{mmpx:%{fcheck-pointer-bounds:    %{static:--whole-archive -lmpx --no-whole-archive %:include(libmpx.spec)%(link_libmpx)}    %{!static:%{static-libmpx:-Bstatic --whole-archive}    %{!static-libmpx:--push-state --no-as-needed} -lmpx     %{!static-libmpx:--pop-state}     %{static-libmpx:--no-whole-archive -Bdynamic %:include(libmpx.spec)%(link_libmpx)}}}}%{mmpx:%{fcheck-pointer-bounds:%{!fno-chkp-use-wrappers:    %{static:-lmpxwrappers}    %{!static:%{static-libmpxwrappers:-Bstatic}    -lmpxwrappers %{static-libmpxwrappers: -Bdynamic}}}}}}} %{mmpx:%{fcheck-pointer-bounds:%{!static:%{m16|m32|mx32:;:-z bndplt }}}}     %{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*:%*} 1):	%:include(libgomp.spec)%(link_gomp)}    %{fcilkplus:%:include(libcilkrts.spec)%(link_cilkrts)}    %{fgnu-tm:%:include(libitm.spec)%(link_itm)}    %(mflib)  %{fsplit-stack: --wrap=pthread_create}    %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} %{!nostdlib:%{!nodefaultlibs:%{%:sanitize(address): %{static-libasan|static:%:include(libsanitizer.spec)%(link_libasan)}    %{static:%ecannot specify -static with -fsanitize=address}}    %{%:sanitize(thread): %{static-libtsan|static:%:include(libsanitizer.spec)%(link_libtsan)}    %{static:%ecannot specify -static with -fsanitize=thread}}    %{%:sanitize(undefined):%{static-libubsan:-Bstatic} %{!static-libubsan:--push-state --no-as-needed} -lubsan  %{static-libubsan:-Bdynamic} %{!static-libubsan:--pop-state} %{static-libubsan|static:%:include(libsanitizer.spec)%(link_libubsan)}}    %{%:sanitize(leak): %{static-liblsan|static:%:include(libsanitizer.spec)%(link_liblsan)}}}}     %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}    %{!nostdlib:%{!nostartfiles:%E}} %{T*}  
%(post_link) }}}}}}

[Igor Engel]

Поправил на заполнение нулями, а ещё нашёл что памяти выделялось сильно больше чем надо.

[Sokolov Viacheslav]

Комментарий не к последней версии.

6 #define HeadersSize? BitmapFileHeaderSize? + BitmapInfoHeaderSize?
подобные конструкции лучше всегда оборачивать в (), чтобы безопасно использовать в любых контекстах. Сейчас HeadersSize? & 1 будет работать не так, как ожидалось.

Если использовать #pragma pack, то почему не использовать его сразу для BitmapFileHeader? и BitmapInfoHeader??

опечатка Invlaid

можно легко сократить пиковое потребление памяти

big-endian не надо. как и случай не twos-complement.

20 if(ret->bfType == 0x4D42 && ret->bfSize >= HeadersSize? && ret->bfReserved1 == 0 && ret->bfReserved2 == 0 && ret->bfOffBits >= HeadersSize?) {
21 return ret;
22 } else {
23 return NULL;
24 }

тут в else не хватает free (и в последующих методах тоже)

58 ret->image_width = bi->biWidth;

тут видно некоторое противоречие в стиле именования

в load_bmp можно за счет порядка операций уменьшить количество кода (например, ret можно сразу аллоцировать)

200 if(bmp->bf != NULL) free(bmp->bf)
во free можно передавать NULL, так что эти if-ы можно удалить

229 if(pinp == NULL

pout == NULL)

кажется, тут лучше поставить assert (если это произошло, то это ошибка программиста)

в rotate_image же может не хватить bmp->meta->size, потому что из-за выравниваний хранимый размер может увеличиться

Кажется, rotate_image делает не совсем поворот.. а скорее даже симметрию..

Обратите внимание на требование

При необходимости выравнивания данных, дописывайте именно нули.

кажется, оно не выполняется сейчас

Мне нравится, что удалось избежать лапши из if-else в main за счет соглашения о поведении разных функций в программе.

В map_char тоже можно же использовать switch?

Кажется, insert_message/extract_message можно было бы реализовать проще (в два прохода по файлу либо считывая очередную порцию ключа по требованию, то есть продвигаясь по файлу с сообщением на 1 символ, а по файлу с ключом - на 5 строк)

[Igor Engel]

Fixed

Переделал

Fixed

Сделал.

ок

вроде везде подобавлял

Поменял нэйминг

Переделал

Убрал ифы

Поставил ассерты

Поправил

Ой... Поправил.

Вроде и так везде было, но после переписывания точно должно быть норм

передлал

insert ещё можно красиво сделать, а вот с экстрактом сложнее - он всё-равно должен будет читать весь ключ, перевыделять сообще, делить что-нибудь на 5 и так далее...

[Sokolov Viacheslav]

In file included from include/stego.h:2:0,
                 from src/main.c:4:
include/bmp.h:8:9: error: unknown type name ‘uint16_t’
 typedef uint16_t WORD;
         ^~~~~~~~
include/bmp.h:9:9: error: unknown type name ‘uint32_t’
 typedef uint32_t DWORD;
         ^~~~~~~~
Makefile:14: recipe for target 'obj/main.o' failed
make: *** [obj/main.o] Error 1

[Igor Engel]

Поправил. По неясным причинам, у меня оно собиралось...

[Sokolov Viacheslav]

Похоже, сейчас все работает, но смотреть я буду на выходных

[Sokolov Viacheslav]

138 if(bmp == NULL || x > bmp->meta->width || y > bmp->meta->height) {

>= же?