Unused option in file copy

[floriangit]

elks/elkscmd/file_utils/cp.c

Line 30 in e9e445d

int opt_nocopyzero;

The option is evaluated but never set. Might either implement the option or delete the code.

[ghaerr]

Nice find! I remember there was at one time a need for this, let me take a look to see what that might have been for. There was a time when we used zero-length '.keep' files in elks/target during the image creation process, but that was moved to image/Makefile handling.

[floriangit]

Yes, it looks like it's getting propagated external in elks/tools/mfs/protos.h and (potentially) used by mkfs.c and genfs.c from the same directory.
I tested prefixing it with static in the original found location just to learn:

• the build went fine nonetheless - indicating I don't use mfs (which seems to be host tools to work on Minix FS)?
• why the compiler cannot emit a warning since it is set-once-but-never-changed now that I made it static?

[ghaerr]

Yes, it looks like it's getting propagated external in elks/tools/mfs/protos.h and (potentially) used by mkfs.c and genfs.c from the same directory.

Thanks for digging that up, I remember what happened now - the code was originally written for mfs to recurse a directory tree for the mfs genfs option which is used to build a MINIX image from the elks/target tree where all applications have been copied. The mkfs -k option sets opt_nocopyzero, which was added so that the zero-length .keep files in some empty elks/target directories would allow the directory to be created even with no files in it (other than .keepm, which isn't copied).

Then, cp was enhanced with the -R option. I copied the recursive directory code from mfs into cp.c, and never implemented the opt_nocopyzero option. It isn't really needed, so should probably be deleted, unless the same code might be used elsewhere, in which case it should be #if UNUSED out. Either is fine with me.

the build went fine nonetheless - indicating I don't use mfs

That code isn't shared, so making the option static didn't change anything with mfs.

why the compiler cannot emit a warning since it is set-once-but-never-changed now that I made it static?

Interestingly enough, I checked the generated code output from the compilation of cp.c (using ia16-elf-objdump -D -r -Mi8086 cp.o > file) when you first asked this question - and sure enough the option value was checked and code generated.

I highly suspect that after you declared the variable static the code was silently deleted by ia16-elf-gcc; this can be checked by running ia16-elf-objdump as above and searching the output for opt_nozerocopy: it will not exist, is my bet, meaning it was eliminated with the dead code elimination pass. The reason this doesn't happen when non-static is the compiler saw a global variable, and can't be sure that another function outside the current compilation unit didn't change that global, so it has to generate code for it. Only the linker knows what is finally included, and our linker doesn't do dead code elimination within .o modules specified in the command line.

Thanks for your investigation. You're welcome to submit a PR to clean this up by deleting the unused code and global if you desire.

[floriangit]

thanks, let me check a bit more.

[floriangit]

I highly suspect that after you declared the variable static the code was silently deleted by ia16-elf-gcc;

Correct. The only way I could have the compiler keep the static-made opt_nocopyzero in the .bss section was to compile with -O0...I'll delete the unused code in a PR

[ghaerr]

Fixed by #2187.