Danek Duvall wrote:
On Tue, Mar 25, 2008 at 01:43:54PM -0700, Danek Duvall wrote:On Tue, Mar 25, 2008 at 01:21:39PM -0700, Danek Duvall wrote:add driver name=dmfe perms="* 0666 root sys" clone_perms="0666 root sys"I think I'd considered this, and rejected it because I wasn't sure if it would *always* match (or if there might ever be a case where a driver would deliver the clone perm for more than one minor node name). If I can confirm that assumption, I'll change the code.Ed confirms that the word following the colon must match the driver name. I'll make the change and send out a new webrev.It's a bit more complicated than that, but the kernel code that parses minor_perm doesn't support the one case (sbpro) where this doesn't apply in the current WOS. So here's the second rev of 780: http://cr.opensolaris.org/~dduvall/pkg-780-2/ which also adds clone_perms to pkg(5). I've been pretty vague in the man page about what values the attributes can take, and remain so.
driver.py
General comments
1) Where a call to subprocess.call fails, I would find it useful to
know what the command was that failed ('args'), not just the driver
name. At least then, someone trying to debug it has a clue about
what to test and it might make the bug report or service ticket
more meaningful ;) I realise that you print out some of the
pertinent information, but I was looking at it from a support
perspective.
2) The code uses the construct:
x = os.path.normpath(os.path.join(img.get_root(), filename))
and
x = file(os.path.normpath(os.path.join(img.get_root(), filename)))
in a number of places. Is it worth putting this into a function
so that if you ever need to change the way it accesses a file
in the image root, you only need to change a small number of places?
3) Do you see any value in putting the file names into string
constants (well, in as much as Python has such a thing) at the
top of the file, since the names are often used more than once?
Lines 188-209 - update etc/driverclasses
1) Should the try/except block be inside a test so that it is only
executed if add_class or rem_class are not None/empty?
2) Would it be useful to also print out the add_class and rem_class
values in the event of failure? I.e. 'this is what I was trying
to change'. Then a user may be able to recover with a manual fix
later.
3) Is it worth separating the read and write of the driverclasses
file into two separate try/except blocks to more accurately
report a failure cause?
Line 290 - def _snarf(...)
No offence intended Danek, but frankly, I'm surprised that you used
a slang name for this function when you seem to be very rigorous
about code style most of the time.
I'm not hung up on changing the name, but _snarf just strikes me
as a little crass, even though I get why you named it thus.
I didn't check all of the package files for correctness.
Trev
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
