Page MenuHomeFreeBSD
Differential D3166

Static some variables and clean up extra new lines to make the code more readable.
ClosedPublic
Actions

Authored by araujo on Jul 23 2015, 4:12 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 8, 8:24 PM
Unknown Object (File)
Wed, Apr 8, 12:08 PM
Unknown Object (File)
Tue, Apr 7, 3:26 AM
Unknown Object (File)
Tue, Apr 7, 1:56 AM
Unknown Object (File)
Mon, Apr 6, 8:03 AM
Unknown Object (File)
Sun, Apr 5, 5:59 PM
Unknown Object (File)
Sat, Apr 4, 5:00 AM
Unknown Object (File)
Sun, Mar 29, 4:54 AM
View All Files
Subscribers
imp

Details

Reviewers
• rodrigc
kib
dim
ngie
Commits
rS285926: Staticfy and constify some variables and clean up the code a bit to make it
Summary

I'm gonna start to update the NIS softwares, this is the first update to make the code a bit more readable.

Also, some tools are from OpenBSD, and I intend to update those for the next couple weeks.

The NIS Server(ypserver), was wrote by wpaul@ and the implementation is a bit different than the one on OpenBSD, in this case, I will rewrite few things, instead of sync with the OpenBSD project.

NOTE: I have contacted wpaul@ and asked him if I can change the license from 4 clauses BSD to a more modern one 2 CLAUSES.
Test Plan

The test were done as following:

  1. Create a NIS server.
  2. Use a NIS client to cat some mapped files such like:

root@srcCODE:~ # ypwhich
10.0.1.11
root@srcCODE:~ # ypwhich -x
"passwd" is an alias for "passwd.byname"
"master.passwd" is an alias for "master.passwd.byname"
"shadow" is an alias for "shadow.byname"
"group" is an alias for "group.byname"
"networks" is an alias for "networks.byaddr"
"hosts" is an alias for "hosts.byaddr"
"protocols" is an alias for "protocols.bynumber"
"services" is an alias for "services.byname"
"aliases" is an alias for "mail.aliases"
"ethers" is an alias for "ethers.byname"

passwd:
root@srcCODE:~ # ypcat -d freebsd passwd
smmsp:*:25:25:Sendmail Submission User:/var/spool/clientmqueue:/usr/sbin/nologin
hast:*:845:845:HAST unprivileged user:/var/empty:/usr/sbin/nologin
bin:*:3:7:Binaries Commands and Source:/:/usr/sbin/nologin
tty:*:4:65533:Tty Sandbox:/:/usr/sbin/nologin
kmem:*:5:65533:KMem Sandbox:/:/usr/sbin/nologin
sshd:*:22:22:Secure Shell Daemon:/var/empty:/usr/sbin/nologin
proxy:*:62:62:Packet Filter pseudo-user:/nonexistent:/usr/sbin/nologin
unbound:*:59:59:Unbound DNS Resolver:/var/unbound:/usr/sbin/nologin
_pflogd:*:64:64:pflogd privsep user:/var/empty:/usr/sbin/nologin
nobody:*:65534:65534:Unprivileged user:/nonexistent:/usr/sbin/nologin
root:*:0:0:Charlie &:/root:/bin/csh
toor:*:0:0:Bourne-again Superuser:/root:
daemon:*:1:1:Owner of many system processes:/root:/usr/sbin/nologin
operator:*:2:5:System &:/:/usr/sbin/nologin
man:*:9:9:Mister Man Pages:/usr/share/man:/usr/sbin/nologin
games:*:7:13:Games pseudo-user:/usr/games:/usr/sbin/nologin
news:*:8:8:News Subsystem:/:/usr/sbin/nologin
bind:*:53:53:Bind Sandbox:/:/usr/sbin/nologin
uucp:*:66:66:UUCP pseudo-user:/var/spool/uucppublic:/usr/local/libexec/uucp/uucico
www:*:80:80:World Wide Web Owner:/nonexistent:/usr/sbin/nologin
mailnull:*:26:26:Sendmail Default User:/var/spool/mqueue:/usr/sbin/nologin
_dhcp:*:65:65:dhcp programs:/var/empty:/usr/sbin/nologin
pop:*:68:6:Post Office Owner:/nonexistent:/usr/sbin/nologin
auditdistd:*:78:77:Auditdistd unprivileged user:/var/empty:/usr/sbin/nologin
git_daemon:*:964:964:git daemon:/nonexistent:/usr/sbin/nologin

hosts:
root@srcCODE:~ # ypcat -d freebsd hosts
::1 localhost localhost.my.domain
127.0.0.1 localhost localhost.my.domain

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

araujo updated this revision to Diff 7210.Jul 23 2015, 4:12 AM
araujo retitled this revision from to Static some variables and clean up extra new lines to make the code more readable..
araujo updated this object.
araujo edited the test plan for this revision. (Show Details)
Herald added a subscriber: imp. · View Herald TranscriptJul 23 2015, 4:12 AM
araujo updated this revision to Diff 7214.Jul 23 2015, 7:04 AM

Forgot to update some other yp* tools.

araujo updated this revision to Diff 7339.Jul 26 2015, 9:02 AM

Fix small space/tab for a comment.

araujo updated this object.Jul 26 2015, 10:09 AM
araujo added reviewers: • rodrigc, dim.
araujo updated this object.
araujo added a reviewer: kib.
araujo edited the test plan for this revision. (Show Details)Jul 26 2015, 10:11 AM
araujo added a reviewer: ngie.
kib edited edge metadata.Jul 27 2015, 2:46 AM

Tests should include retrieval of all yp maps. Do not limit this to the most common, lik passwd and group.

As the very important testing step, compare the symbol table of the pre- and post-patch libc. Diff the sorted output of nm --dynamic on libc.so.7 for start; you might need to strip the first column which contains the symbol address.

usr.bin/ypcat/ypcat.c
52 ↗(On Diff #7339)

While there, could you (try to) constify all the tables ? I think both the array itself could be constifyied, and the char * members could be tried to convert to const char *.

araujo added inline comments.Jul 27 2015, 12:06 PM
usr.bin/ypcat/ypcat.c
52 ↗(On Diff #7339)

It is possible to constify all the tables, but I can't constify the char * members because of the function:

/usr/include/rpcsvc/ypclnt.h:86:34
int yp_all(char *indomain, char *inmap, struct ypall_callback *incallback);

It is possible to convert to const, but then I need to convert that function and touch in other parts of the system.

araujo added a comment.Jul 27 2015, 12:41 PM
In D3166#64420, @kib wrote:

Tests should include retrieval of all yp maps. Do not limit this to the most common, lik passwd and group.

As the very important testing step, compare the symbol table of the pre- and post-patch libc. Diff the sorted output of nm --dynamic on libc.so.7 for start; you might need to strip the first column which contains the symbol address.

I checked using nm(1) and the result I can see all yp_* function that should be visible externally. Here is a wc(1) to count the lines with/without the patch.

root@srcCODE:/usr/src # cat /root/without_patch.txt | wc -l

3116

root@srcCODE:/usr/src # cat /root/with_patch.txt | wc -l

3116
araujo updated this revision to Diff 7379.Jul 27 2015, 12:44 PM
araujo updated this object.
araujo edited the test plan for this revision. (Show Details)
araujo edited edge metadata.

Constify all tables, suggested by @kib.

araujo added a comment.Jul 27 2015, 1:09 PM
In D3166#64420, @kib wrote:

Tests should include retrieval of all yp maps. Do not limit this to the most common, lik passwd and group.

I made the test with all maps exposed by ypwhich -x, I just didn't past here because the output is a bit big.

kib added a comment.Jul 27 2015, 6:09 PM
In D3166#64621, @araujo wrote:

I checked using nm(1) and the result I can see all yp_* function that should be visible externally. Here is a wc(1) to count the lines with/without the patch.

Do diff, it is more reliable than line number calculations, due to pecularities of nm (it could list duplicates, it lists file and section names).

Otherwise I am fine with this.

Closed by commit rS285926: Staticfy and constify some variables and clean up the code a bit to make it (authored by araujo). · Explain WhyJul 28 2015, 2:33 AM
This revision was automatically updated to reflect the committed changes.
araujo mentioned this in rS285926: Staticfy and constify some variables and clean up the code a bit to make it.

Revision Contents
Changeset List

PathSize
head/
libexec/
ypxfr/
4 lines
12 lines
2 lines
usr.bin/
ypcat/
6 lines
ypmatch/
4 lines
ypwhich/
4 lines
usr.sbin/
yp_mkdb/
4 lines
yppush/
24 lines
ypserv/
8 lines
2 lines
6 lines
6 lines

Diff 7408

View Options

head/libexec/ypxfr/ypxfr_getmap.c

Loading...
View Options

head/libexec/ypxfr/ypxfr_main.c

Loading...
View Options

head/libexec/ypxfr/ypxfrd_getmap.c

Loading...
View Options

head/usr.bin/ypcat/ypcat.c

Loading...
View Options

head/usr.bin/ypmatch/ypmatch.c

Loading...
View Options

head/usr.bin/ypwhich/ypwhich.c

Loading...
View Options

head/usr.sbin/yp_mkdb/yp_mkdb.c

Loading...
View Options

head/usr.sbin/yppush/yppush_main.c

Loading...
View Options

head/usr.sbin/ypserv/yp_access.c

Loading...
View Options

head/usr.sbin/ypserv/yp_dblookup.c

Loading...
View Options

head/usr.sbin/ypserv/yp_error.c

Loading...
View Options

head/usr.sbin/ypserv/yp_main.c

Loading...