[Freeipa-devel] [PATCH] #319 better cope with ntp config files
Simo Sorce
ssorce at redhat.com
Fri Oct 15 14:27:59 UTC 2010
On Fri, 15 Oct 2010 10:05:50 -0400
Rob Crittenden <rcritten at redhat.com> wrote:
> Simo Sorce wrote:
> >
> > Instead of replacing the files altogether parse them and add only
> > the options we care about.
> >
> > For ntp.conf those are the server related options.
> > For sysconfig/ntpd we care of adding just -x and -g if missing
> >
> > Simo.
> >
>
> nack, I don't think this will work.
my tests did work, but I haven't tried all possibilities indeed.
> A few comments on the python. You use some C-like syntax with match =
> 0, should probably be match = False (as you use elsewhere, with
> file_changed).
>
> You don't need the string module to use split, so instead of:
>
> opt = string.split(line, " ")
>
> You can do:
>
> opt = line.split(" ")
>
> But what you really want, I think is:
>
> opt = line.split()
ok, will change these.
> If you split on None it splits on white space, not a single space.
> The different is:
>
> >>> line = 'server 127.127.1.0 # local clock'
> >>> line.split()
> ['server', '127.127.1.0', '#', 'local', 'clock']
> >>> line.split(' ')
> ['server', '', '127.127.1.0', '', '', '', '', '#', 'local', 'clock']
>
> Note the extra space after server in the last entry. This would cause
> your conditional to fail (if opt[1] == srv).
Right, thanks for catching this, my python got a bit rusty in the last
few months :)
> I'm not sure your loop for srv actually does the right thing. I
> wonder if you wanted to set match = 0 within the for loop.
Well, according to my testing it does.
But I'll re-check.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
More information about the Freeipa-devel
mailing list