Replace GNU RCS ident with a BSD license ident

[bapt]

Summary

100% compatible with GNU RCS ident version in base with the exception of the -V
version which output nothing in the new version)
100% compatible with GNU RCS 5.9.4 with the exception of long options which has
not been added.
Passes all regression tests from GNU RCS 5.9.4
Support svn extension for the Keyword id (double dots and # before last $)

Rationale: lots of scripts are using ident(1) but if base is built WITHOUT_RCS
then those scripts become unusable.

[emaste]

usr.bin/ident/ident.1

46 ↗

(On Diff #7297)

s/it/are/

51 ↗

(On Diff #7297)

"composed only" and "in the C locale"

[bapt]

fix manpage

[bapt]

@emaste comments addressed

[delphij]

Please see my inline comments. The first one is more important (although I don't see it a big problem in practice), the other two are cosmetic.

In additional to that, looking at your code I found another bug and here is the test case:

# $$FreeBSD: head/COPYRIGHT 276462 2014-12-31 10:00:43Z bz $

usr.bin/ident/ident.c

105 ↗

(On Diff #7298)

I think this is wrong (although the impact is probably small). If we have more than INTMAX worth of ID's, eventually we would get an overflow and therefore get wrong result.

If I was you I would use a boolean or consider nbid as a boolean here, initialize as false, and |= each time, because it's all we care for the purpose of -q and we don't care the count.

113 ↗

(On Diff #7298)

Maybe errx?

139 ↗

(On Diff #7298)

It's not common for system utilities to use errx to give usage information. Is this to match GNU behavior?

(Note that new GNU RCS (5.9.4) only gives the standard invalid option and not usage.)

[eadler]

meta comment: please support long options. we don't have to block the import on them, but we shouldn't remove that functionality

[bapt]

Avoid integer overflow
Handle $ in the keyword as a reinitialisation of the keyword to match GNU ident behaviour

[bapt]

usr.bin/ident/ident.c

114 ↗

(On Diff #7325)

Nope because that will make a GNU ident regression test fail because in this particular case
GNU ident has no colon separation between the program name and the warning word

140 ↗

(On Diff #7325)

Yes I figured that one out, but did not know which one I should mimic: newer version or the one we have in base

[bapt]

In D3200#64100, @eadler wrote:

meta comment: please support long options. we don't have to block the import on them, but we shouldn't remove that functionality

Long option support is undocumented upstream, neither available in base version. only because of --quiet and --help will not imho really make sense.

Note that in recent rcs version -V is deprecated and replaced by --version but none of this option make sense in base

[wblock]

usr.bin/ident/ident.1

37 ↗

(On Diff #7325)

Argument name is missing:

.Op Ar files

43 ↗

(On Diff #7325)

Does not need "the":

in

46 ↗

(On Diff #7325)

Capitalize "if":

If no arguments are passed, then

48 ↗

(On Diff #7325)

passive->active:

parses the standard input.

51 ↗

(On Diff #7325)

Too many "only"s:

must only be composed of alphanumeric values in the C locale, followed by

53 ↗

(On Diff #7325)

No "then":

a space.

55 ↗

(On Diff #7325)

Duplicate .Pp, remove one.

56 ↗

(On Diff #7325)

s/The following/These/

61 ↗

(On Diff #7325)

Do the same thing as the previous sentence:

Do nothing, added for compatibility with GNU ident.

[bapt]

Address @wblock remarks

[bapt]

usr.bin/ident/ident.1

37 ↗

(On Diff #7325)

Nope this is magic :)

.Op Ar
Will generate:
[file ...]

automagically

53 ↗

(On Diff #7325)

No it will be "followed by ':' then a space.

[wblock]

usr.bin/ident/ident.1

38 ↗

(On Diff #7337)

But later you refer to this argument as files. There might be a precedent for how to handle that. On line 44 below, this markup might work:

.Ar file Ns s .

54 ↗

(On Diff #7337)

Okay, replace "then" with "and":

and a space.

Rendering as:
must only be composed of alphanumeric values in the C locale, followed by `:' and a space.

[pfg]

I like it, thanks!
I agree the long options are unnecessary.

FWIW, the OpenBSD version uses these defines, but I don't see that as a showstopper.

#define KEYDELIM	'$'	/* keywords delimiter */
#define VALDELIM	':'	/* values delimiter */