testbot/bin: Standardize handling of the Engine's function arguments and add prototypes.

Francois Gouget fgouget at codeweavers.com
Fri Jun 13 11:20:06 CDT 2014


The prototypes let Perl detect when the wrong number of parameters is passed to these functions. The exception to the rule is the HandleXxx() functions due to the way they are called.
The prototypes also document which arguments are optional.
---
 testbot/bin/Engine.pl                    | 46 +++++++++++++++++---------------
 testbot/lib/WineTestBot/Engine/Events.pm | 12 ++++-----
 testbot/lib/WineTestBot/Engine/Notify.pm | 30 ++++++++++-----------
 3 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/testbot/bin/Engine.pl b/testbot/bin/Engine.pl
index ccaadc1..29dc919 100755
--- a/testbot/bin/Engine.pl
+++ b/testbot/bin/Engine.pl
@@ -56,7 +56,7 @@ use WineTestBot::VMs;
 
 my $RunEngine = 1;
 
-sub FatalError
+sub FatalError(@)
 {
   LogMsg @_;
   LogMsg "Shutdown following a fatal error\n";
@@ -215,7 +215,7 @@ sub Cleanup(;$$)
 }
 
 
-sub HandleShutdown
+sub HandleShutdown($$)
 {
   my ($KillTasks, $KillVMs) = @_;
 
@@ -252,12 +252,12 @@ sub HandleShutdown
   return "1OK\n";
 }
 
-sub HandlePing
+sub HandlePing()
 {
   return "1pong\n";
 }
 
-sub HandleJobStatusChange
+sub HandleJobStatusChange($$$)
 {
   my ($JobKey, $OldStatus, $NewStatus) = @_;
 
@@ -297,9 +297,9 @@ sub HandleJobStatusChange
   return "1OK";
 }
 
-sub HandleJobCancel
+sub HandleJobCancel($)
 {
-  my $JobKey = $_[0];
+  my ($JobKey) = @_;
 
   my $Job = CreateJobs()->GetItem($JobKey);
   if (! $Job)
@@ -327,9 +327,9 @@ sub HandleJobCancel
   return "1OK";
 }
 
-sub HandleJobRestart
+sub HandleJobRestart($)
 {
-  my $JobKey = $_[0];
+  my ($JobKey) = @_;
 
   my $Job = CreateJobs()->GetItem($JobKey);
   if (! $Job)
@@ -357,7 +357,7 @@ sub HandleJobRestart
   return "1OK";
 }
 
-sub HandleRescheduleJobs
+sub HandleRescheduleJobs()
 {
   my $ErrMessage = ScheduleJobs();
   if (defined($ErrMessage))
@@ -368,7 +368,7 @@ sub HandleRescheduleJobs
   return "1OK";
 }
 
-sub HandleVMStatusChange
+sub HandleVMStatusChange($$$)
 {
   my ($VMKey, $OldStatus, $NewStatus) = @_;
 
@@ -392,7 +392,7 @@ sub HandleVMStatusChange
   return "1OK";
 }
 
-sub HandleWinePatchMLSubmission
+sub HandleWinePatchMLSubmission()
 {
   my $dh;
   if (!opendir($dh, "$DataDir/staging"))
@@ -443,7 +443,7 @@ sub HandleWinePatchMLSubmission
   return @ErrMessages ? "0". join("; ", @ErrMessages) : "1OK";
 }
 
-sub HandleWinePatchWebSubmission
+sub HandleWinePatchWebSubmission()
 {
   my $LatestWebPatchId = 0;
   my $Patches = CreatePatches();
@@ -514,15 +514,17 @@ sub HandleWinePatchWebSubmission
   return @ErrMessages ? "0". join("; ", @ErrMessages) : "1OK";
 }
 
-sub HandleGetScreenshot
+sub HandleGetScreenshot($)
 {
+  my ($VMName) = @_;
+
   # Validate VM name
-  if ($_[0] !~ m/^(\w+)$/)
+  if ($VMName !~ m/^(\w+)$/)
   {
     LogMsg "Invalid VM name for screenshot\n";
     return "0Invalid VM name";
   }
-  my $VMName = $1;
+  $VMName = $1;
 
   my $VM = CreateVMs()->GetItem($VMName);
   if (! defined($VM))
@@ -554,7 +556,7 @@ my %Handlers=(
     "winepatchwebsubmission"   => \&HandleWinePatchWebSubmission,
     );
 
-sub HandleClientCmd
+sub HandleClientCmd(@)
 {
   my $Cmd = shift;
 
@@ -565,9 +567,9 @@ sub HandleClientCmd
   return "0Unknown command $Cmd\n";
 }
 
-sub ClientRead
+sub ClientRead($)
 {
-  my $Client = shift;
+  my ($Client) = @_;
 
   my $Buf;
   my $GotSomething = !1;
@@ -596,7 +598,7 @@ checks whether any pending patchsets are now complete and thus can be scheduled.
 =back
 =cut
 
-sub SafetyNet
+sub SafetyNet()
 {
   CheckJobs();
   ScheduleJobs();
@@ -610,9 +612,9 @@ sub SafetyNet
   }
 }
 
-sub PrepareSocket
+sub PrepareSocket($)
 {
-  my $Socket = $_[0];
+  my ($Socket) = @_;
 
   my $Flags = 0;
   if (fcntl($Socket, F_GETFL, $Flags))
@@ -663,7 +665,7 @@ sub REAPER
   $SIG{CHLD} = \&REAPER; # still loathe SysV
 }
 
-sub main 
+sub main()
 {
   my ($Shutdown, $KillTasks, $KillVMs);
   while (@ARGV)
diff --git a/testbot/lib/WineTestBot/Engine/Events.pm b/testbot/lib/WineTestBot/Engine/Events.pm
index 9a8da83..584a7e5 100644
--- a/testbot/lib/WineTestBot/Engine/Events.pm
+++ b/testbot/lib/WineTestBot/Engine/Events.pm
@@ -34,7 +34,7 @@ require Exporter;
 
 my %Events;
 
-sub AddEvent
+sub AddEvent($$$$)
 {
   my ($Name, $Timeout, $Repeat, $HandlerFunc) = @_;
 
@@ -44,21 +44,21 @@ sub AddEvent
                     HandlerFunc => $HandlerFunc};
 }
 
-sub DeleteEvent
+sub DeleteEvent($)
 {
-  my $Name = $_[0];
+  my ($Name) = @_;
 
   delete $Events{$Name};
 }
 
-sub EventScheduled
+sub EventScheduled($)
 {
-  my $Name = $_[0];
+  my ($Name) = @_;
 
   return defined($Events{$Name});
 }
 
-sub RunEvents
+sub RunEvents()
 {
   my $Now = time();
   my $Next = undef;
diff --git a/testbot/lib/WineTestBot/Engine/Notify.pm b/testbot/lib/WineTestBot/Engine/Notify.pm
index d0d4890..3063e99 100644
--- a/testbot/lib/WineTestBot/Engine/Notify.pm
+++ b/testbot/lib/WineTestBot/Engine/Notify.pm
@@ -39,9 +39,9 @@ require Exporter;
 @EXPORT_OK = qw($RunningInEngine);
 
 
-sub SendCmdReceiveReply
+sub SendCmdReceiveReply($)
 {
-  my $Cmd = shift;
+  my ($Cmd) = @_;
 
   if (defined($RunningInEngine))
   {
@@ -74,7 +74,7 @@ sub SendCmdReceiveReply
   return $Reply;
 }
 
-sub Shutdown
+sub Shutdown($$)
 {
   my ($KillTasks, $KillVMs) = @_;
 
@@ -93,13 +93,13 @@ sub Shutdown
   return substr($Reply, 1);
 }
 
-sub PingEngine
+sub PingEngine()
 {
   my $Reply = SendCmdReceiveReply("ping\n");
   return 1 <= length($Reply) && substr($Reply, 0, 1) eq "1";
 }
 
-sub JobStatusChange
+sub JobStatusChange($$$)
 {
   my ($JobKey, $OldStatus, $NewStatus) = @_;
 
@@ -116,9 +116,9 @@ sub JobStatusChange
   return substr($Reply, 1);
 }
 
-sub JobCancel
+sub JobCancel($)
 {
-  my $JobKey = $_[0];
+  my ($JobKey) = @_;
 
   my $Reply = SendCmdReceiveReply("jobcancel $JobKey\n");
   if (length($Reply) < 1)
@@ -133,9 +133,9 @@ sub JobCancel
   return substr($Reply, 1);
 }
 
-sub JobRestart
+sub JobRestart($)
 {
-  my $JobKey = $_[0];
+  my ($JobKey) = @_;
 
   my $Reply = SendCmdReceiveReply("jobrestart $JobKey\n");
   if (length($Reply) < 1)
@@ -150,7 +150,7 @@ sub JobRestart
   return substr($Reply, 1);
 }
 
-sub RescheduleJobs
+sub RescheduleJobs()
 {
   my $Reply = SendCmdReceiveReply("reschedulejobs\n");
   if (length($Reply) < 1)
@@ -165,7 +165,7 @@ sub RescheduleJobs
   return substr($Reply, 1);
 }
 
-sub VMStatusChange
+sub VMStatusChange($$$)
 {
   my ($VMKey, $OldStatus, $NewStatus) = @_;
 
@@ -182,7 +182,7 @@ sub VMStatusChange
   return substr($Reply, 1);
 }
 
-sub WinePatchMLSubmission
+sub WinePatchMLSubmission()
 {
   my $Reply = SendCmdReceiveReply("winepatchmlsubmission\n");
   if (length($Reply) < 1)
@@ -197,7 +197,7 @@ sub WinePatchMLSubmission
   return substr($Reply, 1);
 }
 
-sub WinePatchWebSubmission
+sub WinePatchWebSubmission()
 {
   my $Reply = SendCmdReceiveReply("winepatchwebsubmission\n");
   if (length($Reply) < 1)
@@ -212,9 +212,9 @@ sub WinePatchWebSubmission
   return substr($Reply, 1);
 }
 
-sub GetScreenshot
+sub GetScreenshot($)
 {
-  my $VMName = $_[0];
+  my ($VMName) = @_;
 
   my $Reply = SendCmdReceiveReply("getscreenshot $VMName\n");
   if (length($Reply) < 1)
-- 
2.0.0



More information about the wine-patches mailing list