A couple issues with this.

I disagree about checking against OS_MAX_FILE_NAME for two big reasons.

1. This is a full path name on the HOST machine. The location of the file on the HOST can be very long and this has absolutely no relationship to its location once copied to a target. So warning if the HOST path exceeds OS_MAX_FILE_NAME doesn't add anything useful, it's just noise.

Disagree. Line in build is:

cd /home/jhageman/cFS/cFS-GitHub/build/cpu1/apps/sch_lab/tables_cpu1 && /home/jhageman/cFS/cFS-GitHub/build/tools/elf2cfetbl/elf2cfetbl sch_lab_table.o

This should warn if the name is too long, since the expectation when is this table should be loadable without any changes.

2. This tool should not know what the osconfig.h of the target is, at all. We need to reduce dependencies on target-specific config here (I am going to file a separate bug about the inclusion of cfe_tbl_internal.h which should _not be required after the CFE pull request 27)

It needs to build tables that work with cFE, hence it's by design dependent on cFE settings. Feel free to suggest alternatives, but this warning has purpose.

Also, I believe POSIX specifies that PATH_MAX is defined in limits.h. So one should #include <limits.h> not #include <linux/limits.h>. The former is POSIX-compliant but the latter, of course, will only work on Linux.

Agreed.

This is a full path name on the HOST machine. The location of the file on the HOST can be very long and this has absolutely no relationship to its location once copied to a target. So warning if the HOST path exceeds OS_MAX_FILE_NAME doesn't add anything useful, it's just noise.

Disagree. Line in build is:

cd /home/jhageman/cFS/cFS-GitHub/build/cpu1/apps/sch_lab/tables_cpu1 && /home/jhageman/cFS/cFS-GitHub/build/tools/elf2cfetbl/elf2cfetbl sch_lab_table.o

This should warn if the name is too long, since the expectation when is this table should be loadable without any changes.

That is only an artifact of how the build system happens to currently invoke it as part of the current/default build process, because it "cd'd" into the directory first and then ran the tool so it therefore didn't need to specify any relative path to the file.... If you run this from the command line or otherwise write your own Makefile to generate a table (as many do), or even if the CMake recipe changes in the future then this can be a full path name to the source file.

That particular method of invocation is not a requirement and not guaranteed to remain consistent in the future.

I thought this was the whole reason for changing from OS_MAX_FILE_NAME to PATH_MAX -- because it is a host-side tool and this is storing a host-side path name, so OS_MAX_FILE_NAME is irrelevant here.

This tool should not know what the osconfig.h of the target is, at all. We need to reduce dependencies on target-specific config here (I am going to file a separate bug about the inclusion of cfe_tbl_internal.h which should _not be required after the CFE pull request 27)

It needs to build tables that work with cFE, hence it's by design dependent on cFE settings. Feel free to suggest alternatives, but this warning has purpose.

But CFE doesn't have a definition for this, OSAL does. And it can be different on every CPU, and elf2cfetbl doesn't know which CPU it is actually generating a table for.

That particular method of invocation is not a requirement and not guaranteed to remain consistent in the future.
...
I thought this was the whole reason for changing from OS_MAX_FILE_NAME to PATH_MAX -- because it is a host-side tool and this is storing a host-side path name, so OS_MAX_FILE_NAME is irrelevant here.
...
But CFE doesn't have a definition for this, OSAL does. And it can be different on every CPU, and elf2cfetbl doesn't know which CPU it is actually generating a table for.

Understood it's not perfect, and if build system or invocation changes it may be a false warning but in our current context it's useful for us and not an error.

EDIT: By "not an error" I mean feel free to > /dev/null if you want to ignore it, it'll still work.

It is more than just not perfect, it's not correct. Perhaps I didn't express that clearly enough...

• It is validating/checking something that happens to work today only by mere coincidence.
• It is checking both the source and destination files, where only the destination file name length even remotely matters (because this is the one that gets put on the target, not the source file).
• It is checking the entire path name against OS_MAX_FILE_NAME -- which is wrong -- this is a path name so it should be OS_MAX_PATH_NAME (but not, because its a host path name)
• The actual file name part (the one that would be validated against OS_MAX_FILE_NAME) is ALREADY guaranteed to be less than the maximum size, because it is stored in the TblFileDef object in a limited-size buffer.

In other words, the validation you are looking for (that the actual filename part is less than the size limit) is already done, and this extra check is both superfluous and wrong.

To make it even more problematic, this will start to trigger a compiler error once merged with the other cleanup, when you (rightfully) cannot include osconfig.h anymore, because the hacks are taken out.

• The actual file name part (the one that would be validated against OS_MAX_FILE_NAME) is ALREADY guaranteed to be less than the maximum size, because it is stored in the TblFileDef object in a limited-size buffer.

Good point. I was thrown off by the destination directory command line option, which just gets appended to.

In other words, the validation you are looking for (that the actual filename part is less than the size limit) is already done...

And that's what I needed to hear! I'm on board with just taking out the warnings now.

removed warning, osconfig.h and changed to linux.h. Tested with sample_table, still work.

update to path_max -1