Ship It!

We often distinguish between a fatal error and an error for usage. Presumably we should here too?

Rather than cast zero to a pointer type, why not just use NULL, and then you shouldn't need the pointer either.

Also, this returns a pointer type, see head/xlocale.h, so don't do a comparison with zero.

Please give the user a more actionable error message. If I saw an error message that was something like:

'demangle: newlocale: no such file or directory' I would have no idea what to do. At least something like, 'failed to construct the C locale: <error string>' would be a bit more useful and would speed up the debugging process/confusion.

Should we use warnx here for consistency of having the program name at the start?

Presumably i should match the type of argc, which is an int?

Should we warn at all to stderr about failures to demangle a name?

It's probably worth a comment or explanation of how this heuristic was determined. Without context, it seems arbitrary and it's not clear why certain characters are there and not others.

Please give the user a better error message here and all the other uses of err in this function. I'm only commenting on it here.

Shouldn't we be just checking that fprintf returned -1? Also, since you aren't flushing here and are relying on something else to the -1, I'm not sure why you'd check ferror, given that most of the time it probably will be buffered and if not, then the fprintf should propagate the error, no?

Shouldn't symbol be in a .Ar macro?

I don't think launching into a discussion of what a mangled symbol is right at the start is necessarily the best way to get going. I probably would start with a summary of what the program actually does, like you have at line 46-49. I think that's a bit more user friendly, otherwise what this does is buried behind an explanation of symbol mangling. I'm assuming you did it this way because that's how the GNU c++filt manual is written.

You should refer to the actual operand in this description to refer back to what's in the synopsis.

Most manuals spell out stdin to be 'standard in' rather than use the the C symbol name.

Same thing with stdout and 'standard output'.

Conventionally this uses .Sh, not .Ss.

I don't think it quite actually causes us to only demangle symbols from the language. Instead, it always tries to interpret everything as being from the language. If we had names that are valid in both, then we would interpret it in the specified one, which doesn't quite match what that sentence is saying to me.

The xxxx for this looks a little weird at first glance as it makes it seem like it was a fill me in and not trying to be a four character value.

I would give a brief explanation of these. For example, for rust I would describe which of its mangling formats are supported and I would explain that auto attempts to figure out what i tis automatically.

We normally don't put examples in these kinds of lists. Take a look at sleep(1) for an example.

With each example you should have a textual description of what it is demonstrating.

Should readers be referred to any other utilities in a SEE ALSO or pages?

We should probably explicitly call out the stability of this, in particular that the options are probably not stable and the output format isn't committed.

Sorry for the delay in getting back to this. I saw that there were a number of unresolved things related to non-questions which is why I hadn't looked at the new version. It wasn't clear to me if you had wanted additional feedback or not. I've left follow up on the original feedback and a few more additional things I've noticed below.

So if we fail for some other reason, should we still output what the original symbol was for consistency in addition to the warning or is that not necessary?

I think you probably want to phrase this more concretely as:

.Sh INTERFACE STABILITY
The command line options are
.Sy Uncommitted .
The output format is
.Sy Not-an-Interface .

We should use the standard names here from attributes(5).

Given that you've changed how the symbol version works and warns, should the custr version match?

Thanks for working through this stuff. Seems reasonable.

Is it worth mentioning here that symbol boundaries are only determined by the C locale or is that too much?

Ship It!