Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(136)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 181097: code review 181097: Allow Hostname to return FQDN when /bin/hostname does not

Can't Edit
Can't Publish+Mail
Start Review
Created:
16 years ago by icarus
Modified:
3 years, 9 months ago
Reviewers:
rsc1
CC:
rsc, golang-dev
Visibility:
Public.
Allow Hostname to return FQDN when /bin/hostname does not Hostname reads the file /proc/sys/kernel/hostname to determine the value it returns. Some people set this to a Fully Qualified Doamin Name. At least one implementation of /bin/hostname truncates the name it gets (often from the "uname" system call) at the first dot unless it is given a "-f" flag. This change makes the unit test also truncate at the first dot and checks if the strings then match. This seems more portable than adding an extra flag to the called /bin/hostname program.

Patch Set 1 #

Patch Set 2 : code review 181097: Allow Hostname to return FQDN when /bin/hostname does not #

Total comments: 1

Patch Set 3 : code review 181097: Allow Hostname to return FQDN when /bin/hostname does not #

Created: 16 years ago
Download [raw] [tar.bz2]
Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -1 line) Patch
M src/pkg/os/os_test.go View 1 2 1 chunk +6 lines, -1 line 0 comments Download
Total messages: 11
|
icarus
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review the following change.
16 years ago (2009年12月29日 21:31:05 UTC) #1
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
I'd like you to review the following change.
Sign in to reply to this message.
iant
What system do you notice this problem on? http://codereview.appspot.com/181097/diff/1001/4 File src/pkg/os/os_test.go (right): http://codereview.appspot.com/181097/diff/1001/4#newcode647 src/pkg/os/os_test.go:647: func ...
16 years ago (2009年12月31日 00:14:59 UTC) #2
What system do you notice this problem on?
http://codereview.appspot.com/181097/diff/1001/4
File src/pkg/os/os_test.go (right):
http://codereview.appspot.com/181097/diff/1001/4#newcode647
src/pkg/os/os_test.go:647: func strchr(s string, c uint8) int {
Use string.Index instead.
Sign in to reply to this message.
icarus
On 2009年12月31日 00:14:59, iant wrote: > What system do you notice this problem on? Debian ...
16 years ago (2009年12月31日 01:37:43 UTC) #3
On 2009年12月31日 00:14:59, iant wrote:
> What system do you notice this problem on?
Debian 386, where hostname is set to "xxx.yy.com". This is the value in
/proc/sys/kernel/hostname. "/bin/hostname" returns "xxx", but "/bin/hostname -f"
returns "xxx.yy.com".
So either adding a "-f" or truncating at the "." will make the unit test work.
Truncating seems a more portable solution.
> http://codereview.appspot.com/181097/diff/1001/4
> File src/pkg/os/os_test.go (right):
> 
> http://codereview.appspot.com/181097/diff/1001/4#newcode647
> src/pkg/os/os_test.go:647: func strchr(s string, c uint8) int {
> Use string.Index instead.
Thanks for the comment. It wasn't clear how self contained one should be making
the file. The other uses of the strings package was just the Bytes function.
Sign in to reply to this message.
cw
I think perhaps we should introduce os.Uname and plumb that in. For linux it's a ...
16 years ago (2009年12月31日 03:35:43 UTC) #4
I think perhaps we should introduce os.Uname and plumb that in.
For linux it's a syscall wrapper, darwin will be more or less the same (the
syscall isn't exposed presently though) and sysctl for freebsd.
At which point we can avoid /bin/hostname entirely.
Sign in to reply to this message.
dho
2009年12月30日 <cw@f00f.org>: > I think perhaps we should introduce os.Uname and plumb that in. > ...
16 years ago (2009年12月31日 03:43:00 UTC) #5
2009年12月30日 <cw@f00f.org>:
> I think perhaps we should introduce os.Uname and plumb that in.
>
> For linux it's a syscall wrapper, darwin will be more or less the same
> (the syscall isn't exposed presently though) and sysctl for freebsd.
>
> At which point we can avoid /bin/hostname entirely.
I was under the impression that darwin uses sysctls as well for this.
Either way, yes -- I think this interface should go into os, as it's
not a syscall everywhere, and that would address portability concerns.
--dho
Sign in to reply to this message.
rsc
16 years ago (2010年01月05日 22:48:44 UTC) #6
Sign in to reply to this message.
rsc
On 2009年12月31日 03:35:43, cw wrote: > I think perhaps we should introduce os.Uname and plumb ...
16 years ago (2010年01月08日 08:24:05 UTC) #7
On 2009年12月31日 03:35:43, cw wrote:
> I think perhaps we should introduce os.Uname and plumb that in.
there's no point. the struct passed to uname has
unspecified array lengths in it, and the man page
says that reading /proc/sys/kernel/hostname is
equivalent to getting it from uname.
Sign in to reply to this message.
rsc
On 2009年12月29日 21:31:05, icarus wrote: > Hello mailto:golang-dev@googlegroups.com (cc: mailto:golang-dev@googlegroups.com), > > I'd like you ...
16 years ago (2010年01月08日 08:27:47 UTC) #8
On 2009年12月29日 21:31:05, icarus wrote:
> Hello mailto:golang-dev@googlegroups.com (cc:
mailto:golang-dev@googlegroups.com),
> 
> I'd like you to review the following change.
This looks good, but could you please complete the CLA as
described at http://golang.org/doc/contribute.html#copyright ?
Thanks.
Sign in to reply to this message.
icarus
On 2010年01月08日 08:27:47, rsc wrote: > On 2009年12月29日 21:31:05, icarus wrote: > > Hello mailto:golang-dev@googlegroups.com ...
15 years, 12 months ago (2010年01月11日 23:34:12 UTC) #9
On 2010年01月08日 08:27:47, rsc wrote:
> On 2009年12月29日 21:31:05, icarus wrote:
> > Hello mailto:golang-dev@googlegroups.com (cc:
> mailto:golang-dev@googlegroups.com),
> > 
> > I'd like you to review the following change.
> 
> This looks good, but could you please complete the CLA as
> described at http://golang.org/doc/contribute.html#copyright ?
Done. (As an individual CLA).
Sign in to reply to this message.
rsc1
LGTM catching up on code reviews, sorry for the delay.
15 years, 11 months ago (2010年01月26日 20:36:52 UTC) #10
LGTM
catching up on code reviews, sorry for the delay.
Sign in to reply to this message.
rsc
*** Submitted as http://code.google.com/p/go/source/detail?r=cb37c45d40ae *** os: in test, allow Hostname to return FQDN even if ...
15 years, 11 months ago (2010年01月26日 21:16:05 UTC) #11
*** Submitted as http://code.google.com/p/go/source/detail?r=cb37c45d40ae ***
os: in test, allow Hostname to return FQDN even if /bin/hostname does not
Hostname reads the file /proc/sys/kernel/hostname to determine
the value it returns. Some people set this to a Fully Qualified
Doamin Name. At least one implementation of /bin/hostname
truncates the name it gets (often from the "uname" system call)
at the first dot unless it is given a "-f" flag. This change makes
the unit test also truncate at the first dot and checks if the strings
then match. This seems more portable than adding an extra flag
to the called /bin/hostname program.
R=rsc
CC=golang-dev
http://codereview.appspot.com/181097
Committer: Russ Cox <rsc@golang.org>
Sign in to reply to this message.
|
This is Rietveld f62528b

AltStyle によって変換されたページ (->オリジナル) /