Page MenuHomeFreeBSD

Replace GNU RCS ident with a BSD license ident
ClosedPublic

Authored by bapt on Jul 25 2015, 3:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 20, 2:21 PM
Unknown Object (File)
Thu, Nov 14, 1:19 AM
Unknown Object (File)
Mon, Nov 11, 8:43 AM
Unknown Object (File)
Oct 17 2024, 4:20 AM
Unknown Object (File)
Oct 17 2024, 4:20 AM
Unknown Object (File)
Oct 17 2024, 4:20 AM
Unknown Object (File)
Oct 17 2024, 4:19 AM
Unknown Object (File)
Oct 17 2024, 4:19 AM
Subscribers

Details

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.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Warnings
SeverityLocationCodeMessage
Warningusr.bin/ident/tests/ident.sh:CHMOD1Invalid Executable
Unit
No Test Coverage

Event Timeline

bapt retitled this revision from to Replace GNU RCS ident with a BSD license ident.
bapt updated this object.
bapt edited the test plan for this revision. (Show Details)
bapt added reviewers: emaste, delphij.
usr.bin/ident/ident.1
47

s/it/are/

52

"composed only" and "in the C locale"

bapt edited edge metadata.

fix manpage

bapt marked 2 inline comments as done.Jul 25 2015, 3:32 PM

@emaste comments addressed

delphij requested changes to this revision.EditedJul 25 2015, 7:07 PM
delphij edited edge metadata.

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

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

Maybe errx?

139

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.)

This revision now requires changes to proceed.Jul 25 2015, 7:07 PM

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

bapt edited edge metadata.

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

bapt marked an inline comment as done.Jul 25 2015, 7:55 PM
bapt added inline comments.
usr.bin/ident/ident.c
115

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

141

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

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 added inline comments.
usr.bin/ident/ident.1
38

Argument name is missing:

.Op Ar files

44

Does not need "the":

in

47

Capitalize "if":

If no arguments are passed, then

49

passive->active:

parses the standard input.

52

Too many "only"s:

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

54

No "then":

a space.

56

Duplicate .Pp, remove one.

57

s/The following/These/

62

Do the same thing as the previous sentence:

Do nothing, added for compatibility with GNU ident.

bapt edited edge metadata.

Address @wblock remarks

bapt marked 7 inline comments as done.Jul 26 2015, 1:11 AM
bapt added inline comments.
usr.bin/ident/ident.1
38

Nope this is magic :)

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

automagically

54

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

usr.bin/ident/ident.1
39

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 .

55

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 edited edge metadata.

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 */
This revision was automatically updated to reflect the committed changes.