diff options
Diffstat (limited to 'zarb-ml/mageia-dev/2012-November/019816.html')
| -rw-r--r-- | zarb-ml/mageia-dev/2012-November/019816.html | 177 |
1 files changed, 177 insertions, 0 deletions
diff --git a/zarb-ml/mageia-dev/2012-November/019816.html b/zarb-ml/mageia-dev/2012-November/019816.html new file mode 100644 index 000000000..c1a06c011 --- /dev/null +++ b/zarb-ml/mageia-dev/2012-November/019816.html @@ -0,0 +1,177 @@ +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN"> +<HTML> + <HEAD> + <TITLE> [Mageia-dev] [soft-commits] [6416] Initial commit of Admin Panel. + </TITLE> + <LINK REL="Index" HREF="index.html" > + <LINK REL="made" HREF="mailto:mageia-dev%40mageia.org?Subject=Re%3A%20%5BMageia-dev%5D%20%5Bsoft-commits%5D%20%5B6416%5D%20Initial%20commit%20of%20Admin%0A%09Panel.&In-Reply-To=%3C50992DBC.9030808%40gmail.com%3E"> + <META NAME="robots" CONTENT="index,nofollow"> + <META http-equiv="Content-Type" content="text/html; charset=us-ascii"> + <LINK REL="Previous" HREF="019817.html"> + <LINK REL="Next" HREF="019820.html"> + </HEAD> + <BODY BGCOLOR="#ffffff"> + <H1>[Mageia-dev] [soft-commits] [6416] Initial commit of Admin Panel.</H1> + <B>Matteo Pasotti</B> + <A HREF="mailto:mageia-dev%40mageia.org?Subject=Re%3A%20%5BMageia-dev%5D%20%5Bsoft-commits%5D%20%5B6416%5D%20Initial%20commit%20of%20Admin%0A%09Panel.&In-Reply-To=%3C50992DBC.9030808%40gmail.com%3E" + TITLE="[Mageia-dev] [soft-commits] [6416] Initial commit of Admin Panel.">matteo.pasotti at gmail.com + </A><BR> + <I>Tue Nov 6 16:33:16 CET 2012</I> + <P><UL> + <LI>Previous message: <A HREF="019817.html">[Mageia-dev] [soft-commits] [6416] Initial commit of Admin Panel. +</A></li> + <LI>Next message: <A HREF="019820.html">[Mageia-dev] [soft-commits] [6416] Initial commit of Admin Panel. +</A></li> + <LI> <B>Messages sorted by:</B> + <a href="date.html#19816">[ date ]</a> + <a href="thread.html#19816">[ thread ]</a> + <a href="subject.html#19816">[ subject ]</a> + <a href="author.html#19816">[ author ]</a> + </LI> + </UL> + <HR> +<!--beginarticle--> +<PRE>-----BEGIN PGP SIGNED MESSAGE----- +Hash: SHA1 + +On 06/11/2012 13:16, Guillaume Rousse wrote: +><i> information in every single file, whereas a single top-level README +</I>><i> file would be enough. +</I>><i> +</I>Hello Guillaume, + +GPLv2 says: +It is safest to attach them to the start of each source file to most +effectively convey the exclusion of warranty; and each file should +have at least the "copyright" line and a pointer to where the full +notice is found. + +<A HREF="http://www.gnu.org/licenses/gpl-2.0.html">http://www.gnu.org/licenses/gpl-2.0.html</A> + +>><i> + +package Auth; +</I>><i> I'm convinced tough than using a shared top-level namespace, for +</I>><i> instance AdminPanel or Mageia::AdminPanel, would be a better idea +</I>><i> to express the idea than this module is a part of a software, than +</I>><i> a loose comment such as "This file is part of mcc2". package +</I>><i> Mageia::AdminPanel::Auth; +</I>><i> +</I>It's just a prototype, it will be "fixed" in future. +>><i> + +require Exporter; <A HREF="https://www.mageia.org/mailman/listinfo/mageia-dev">+ at ISA</A> = qw(Exporter); +</I>><i> You'd rather use a modern perl idiom: use base qw(Exporter) +</I>><i> +</I>><i> or +</I>><i> +</I>><i> use parent qw(Exporter); +</I>><i> +</I>Got it. +>><i> <A HREF="https://www.mageia.org/mailman/listinfo/mageia-dev">+ at EXPORT</A> = qw(require_root_capability + +</I>>><i> ask_for_authentication); + +use strict; +use warnings; +use +</I>>><i> diagnostics; +</I>><i> Those pragmas should come first, before package variables +</I>><i> +</I>Got it +>><i> +use Data::Dumper; +</I>><i> Unused anywere. Don't load debug-related modules in production +</I>><i> coed. +</I>><i> +</I>afaik, apanel is not "in production" so what's your concern?. +>><i> +sub require_root_capability { + return 0 if(!$>); + return +</I>>><i> 1; +} +</I>><i> Perl best practice: use english name for magic variables, for +</I>><i> readability: +</I>><i> +</I>><i> use English qw(-no_match_vars ); return 0 if (!$EUID); +</I>><i> +</I>><i> And your condition could be expressed in a single statement: sub +</I>><i> require_root_capability { return $EUID == 0; } +</I>><i> +</I>>><i> + +sub ask_for_authentication { + my @args = @ARGV; + my +</I>>><i> $command = wrap_command($0); + unshift(@args, $command->[2]); +</I>>><i> + exec { $command->[0] } $command->[1], @args or die ("command +</I>>><i> %s missing", $command->[0]); + die "You must be root to run +</I>>><i> this program" if $>; +} +</I>><i> You're duplicating the condition from previous function here. die +</I>><i> "You must be root to run this program" if +</I>><i> !require_root_capability(); +</I>><i> +</I>Don't blame me too much for readability and duplications, please. +I was inspired by /usr/lib/libDrakX/common.pm and other modules that +are not very readable and that contain similar duplications. +Again, this module it's still a prototype and I'm learning new stuff +while coding it. I'll work on improving its readability. +><i> Morevoer, you'd better test before executing the command: die "You +</I>><i> must be root to run this program" if !require_root_capability(); +</I>><i> exec { $command->[0] } $command->[1], @args or die ("command %s +</I>><i> missing", $command->[0]); +</I>><i> +</I>Same as above, take a look at /usr/lib/libDrakX/common.pm and you'll +see that the test is performed after the exec. +>><i> +sub wrap_command { + my $currenv = "env"; + my $wrapper = +</I>>><i> "pkexec"; + my $app = $0; + my $command = [$wrapper, +</I>>><i> $currenv, $app]; + ($command); +} +</I>><i> Perl best practice: use explicit return statement, for better +</I>><i> readability: return ($command); +</I>><i> +</I>Again, same as above. +><i> Using temporary variables for constant isn't very useful here, the +</I>><i> whole function would probably be more readable this way: +</I>><i> +</I>><i> sub wrap_command { my ($app) = @_; return (['pkexec', 'env', +</I>><i> $app]); } +</I>><i> +</I>><i> I don't understand the need for list contexte here, tough. +</I>><i> +</I>><i> BTW, your indentation isn't consistent between various files. +</I>I worked almost exclusively on the file you have dissected =) and +honestly it's not a big issue to me to read source files with +different indentations (I mean if they are somehow indented and +readable). + +I'll try to improve my code even paying attention to your precious +advices. + +Thank you. + +Matteo +-----BEGIN PGP SIGNATURE----- +Version: GnuPG v1.4.12 (GNU/Linux) +Comment: Using GnuPG with Mozilla - <A HREF="http://enigmail.mozdev.org/">http://enigmail.mozdev.org/</A> + +iQEcBAEBAgAGBQJQmS25AAoJED3LowjDDWbNj3MH/jazMliy7g+VmvYYm5EA2t3G +nqv4o3+FT01cRp45OXq+Xus7TIg/IEFDAtfsIRfWUWH7sH7J1ooWywzourD4kkj0 +XXWw7Wykhhpn2cJ5Sv55KwAr5u+ubt5CC2Tga7sis3hvcsFuMKXOoxV6BtUIUQ4O ++IuzybR7GhjM/B+oGiPdSLQyRXEB/LhxWsle+Xs0Ode5dYjLZ3QR4YumVC6YiYsS +qU/B9SYd3P09K48zqjWNtxDfwf8QItU5oCZ8p9nFnY3vLkW16FGiG2MFJigrOfjq ++Ae+6qH/Q/ZH3iqoZDBGXutr6zUIHhyJmU6ICawR2IIbj6lGdKxTUjqu3aN0H+o= +=U5sK +-----END PGP SIGNATURE----- +</PRE> + + + + + + + + + +<!--endarticle--> + <HR> + <P><UL> + <!--threads--> + <LI>Previous message: <A HREF="019817.html">[Mageia-dev] [soft-commits] [6416] Initial commit of Admin Panel. +</A></li> + <LI>Next message: <A HREF="019820.html">[Mageia-dev] [soft-commits] [6416] Initial commit of Admin Panel. +</A></li> + <LI> <B>Messages sorted by:</B> + <a href="date.html#19816">[ date ]</a> + <a href="thread.html#19816">[ thread ]</a> + <a href="subject.html#19816">[ subject ]</a> + <a href="author.html#19816">[ author ]</a> + </LI> + </UL> + +<hr> +<a href="https://www.mageia.org/mailman/listinfo/mageia-dev">More information about the Mageia-dev +mailing list</a><br> +</body></html> |