1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
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>
|
