code review 4609047: syscall: use strict in perl scripts

syscall: use strict in perl scripts

[mattn, 2011-06-15 05:10:23]

Hello golang-dev@googlecode.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
http://go.googlecode.com/hg/

[bradfitz, 2011-06-15 05:15:02]

On Tue, Jun 14, 2011 at 10:10 PM, <mattn.jp@gmail.com> wrote:

> Reviewers: golang-dev_googlecode.com,
>
> Message:
> Hello golang-dev@googlecode.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> http://go.googlecode.com/hg/
>
>
> Description:
> syscall: make *.pl strictly.
>

syscall: use strict in perl scripts


>
> Please review this at
http://codereview.appspot.com/**4609047/<http://codereview.appspot.com/4609047/>
>
> Affected files:
>  M src/pkg/syscall/mksyscall.pl


Did you forget the "use strict" in this file?  You did it everywhere else,
and fixed the violations here.


>
>  M src/pkg/syscall/mksyscall_**windows.pl <http://mksyscall_windows.pl>
>  M src/pkg/syscall/mksysnum_**darwin.pl <http://mksysnum_darwin.pl>
>  M src/pkg/syscall/mksysnum_**freebsd.pl <http://mksysnum_freebsd.pl>
>  M src/pkg/syscall/mksysnum_**linux.pl <http://mksysnum_linux.pl>
>
>
>

[mattn, 2011-06-15 05:17:36]

Hello golang-dev@googlecode.com, bradfitz@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2011-06-15 17:43:07]

http://codereview.appspot.com/4609047/diff/11001/src/pkg/syscall/mksyscall.pl
File src/pkg/syscall/mksyscall.pl (right):

http://codereview.appspot.com/4609047/diff/11001/src/pkg/syscall/mksyscall.pl...
src/pkg/syscall/mksyscall.pl:31: if(@ARGV) {
You're doing this to work around your "use warnings" addition complaining about
"Use of uninitialized value $ARGV[0]" below, yet you still shift @ARGV (on lines
34, 37, 31, 45) and proceed to access $ARGV[0] which may then be uninitialized. 
(on lines 39, 43, 48)

So this doesn't go far enough.

I think you should just undo this part of the change and remove "use warnings",
else just use the standard library module Getopt::Long and remove all this code.

*shrug*

[rsc, 2011-06-15 20:38:05]

Please just drop use warnings.

[bradfitz, 2011-06-16 00:23:14]

Just remove all the "use warnings" line and remove the line:

if(@ARGV) {

and unindent it all, like it was before.

Just keep the "use strict" lines and the "my" additions.

On Wed, Jun 15, 2011 at 5:14 PM, mattn <mattn.jp@gmail.com> wrote:

> Russ, Do you mean that 'back the code of manipulating @ARGV and remove "use
> warnings"'?
>
>

[mattn, 2011-06-16 00:26:47]

On 2011/06/16 00:23:14, bradfitz wrote:
> Just remove all the "use warnings" line and remove the line:
> 
> if(@ARGV) {
> 
> and unindent it all, like it was before.
> 
> Just keep the "use strict" lines and the "my" additions.
> 
> On Wed, Jun 15, 2011 at 5:14 PM, mattn <mailto:mattn.jp@gmail.com> wrote:
> 
> > Russ, Do you mean that 'back the code of manipulating @ARGV and remove "use
> > warnings"'?
> >
> >

Done

[bradfitz, 2011-06-16 00:36:00]

LGTM

On Wed, Jun 15, 2011 at 5:26 PM, <mattn.jp@gmail.com> wrote:

> On 2011/06/16 00:23:14, bradfitz wrote:
>
>> Just remove all the "use warnings" line and remove the line:
>>
>
>  if(@ARGV) {
>>
>
>  and unindent it all, like it was before.
>>
>
>  Just keep the "use strict" lines and the "my" additions.
>>
>
>  On Wed, Jun 15, 2011 at 5:14 PM, mattn <mailto:mattn.jp@gmail.com>
>>
> wrote:
>
>  > Russ, Do you mean that 'back the code of manipulating @ARGV and
>>
> remove "use
>
>> > warnings"'?
>> >
>> >
>>
>
> Done
>
>
>
http://codereview.appspot.com/**4609047/<http://codereview.appspot.com/4609047/>
>

[bradfitz, 2011-06-16 00:41:14]

*** Submitted as http://code.google.com/p/go/source/detail?r=bd35e21c471e ***

syscall: use strict in perl scripts

R=golang-dev, bradfitz, rsc
CC=golang-dev
http://codereview.appspot.com/4609047

Committer: Brad Fitzpatrick <bradfitz@golang.org>