Add output-dir-mirror option

[xxie24]

No description provided.

[Cyan4973]

Are there cases where the return pointer of this function must be able to mutate or free() its content ?
I did not notice one. If not, it would be better to return a const char*.
As a consequence, the argument could be const char* too, since it's not mutated either.
It also makes it a bit clearer that this function returns a reference, and not a new object.

[xxie24]

UTIL_trimPath will call trimLeadingCurrentDir(), trimLeadingCurrentDir() has user case of returning mutable object.

It remind me the 'strchr()': (https://opensource.apple.com/source/BerkeleyDB/BerkeleyDB-18/db/clib/strchr.c.auto.html)

char *strchr(const char *p, int ch)
{
	char c;

	c = ch;
	for (;; ++p) {
		if (*p == c)
			return ((char *)p);
		if (*p == '\0')
			return (NULL);
	}
	/* NOTREACHED */
}

So seems like even C library is doing the const cast. There is an interesting analysis on this: https://stackoverflow.com/questions/14367727/how-does-strchr-implementation-work. So I can either write 1 function doing const-cast, or write a duplicated functions.

trimLeadingCurrentDir() is 1 liner function, so I probably going to duplicate it into 2 funcitons that return const and non-const type.

[Cyan4973]

Indeed, strchr() is one of these cases where the standard got it seriously wrong, sacrificing the future for the short-term benefit of existing code. That being said, I'm a bit more sympathetic for errors made during this era, when many concepts where still being worked on. After all, const was introduced after strchr(), and this lead to a situation where the function was already deployed, and the committee was under strong pressure to limit code breakage, so adaptation to const was a tricky situation.

Anyway, nowadays, we have no such excuse. const is an extremely important property, and it shall be applied and respected scrupulously. If there is a need for 2 versions, one safe which only works on references and provides references, and a more powerful but also more dangerous one which allows full ownership transfer, then it's better to write 2 functions.

[xxie24]

I ended up doing a trick on this:

static const char*
trimLeadingCurrentDirConst(const char *filename)
{
    assert(filename != NULL); // don't expect null string here
    if ((filename[0] == '.') && (filename[1] == PATH_SEP))
        return filename + 2;
    return filename;
}

static char*
trimLeadingCurrentDir(char *filename)
{
    /* 'union charunion' can do const-cast without compiler warning */
    union charunion {
        char *chr;
        const char* cchr;
    } ptr;
    ptr.cchr = trimLeadingCurrentDirConst(filename);
    return ptr.chr;
}

seems to me a good compromise on this. Let me know what you think.

[Cyan4973]

Interesting. That's a use of union I'm not used to. I'm wondering if it could be prone to strict aliasing issues. If not, then it's a clever use to circumvent const property.

[Cyan4973]

C90 comment style is limited to /* (...) */

[xxie24]

done.

[Cyan4973]

Thanks @xxie24 ,
this is a rather good beginning.
You managed to integrate this capability in a way which makes sense within this project.
Also, the way you divide the topic into unitary components is appropriate.

We'll have a number of back and forth for the tactical details,
mostly around code conventions and specific precautions to C programming,
but I believe you've got the overall strategy right.

[Cyan4973]

Some minor details about error case.
Overall, this looks good, well structured.

Also, consider documenting programs/zstd.1.md.
It's a perfect place to present --output-dir-mirror, and explain how it's supposed to work, and its limitations.

This markdown document will be later translated into a man page (you don't have to do that part, it requires some dependencies).

[Cyan4973]

Thanks @xxie24 , this is a great feature, which is highly welcomed.

I note that one last test is failing (min-decomp-macros),
though I somewhat remember that it might be one of the new tests that tend to be a bit flackey. cc @bimbashrestha