This replaces a lot of direct field accesses. Also clearly identify unused variables.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/web/Submit.pl | 100 ++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 63 deletions(-)
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl index c5423c7951..4368aeeb30 100644 --- a/testbot/web/Submit.pl +++ b/testbot/web/Submit.pl @@ -77,16 +77,14 @@ sub _Upload($) { umask($OldUMask); delete $self->{FileName}; - $self->{ErrField} = "File"; - $self->{ErrMessage} = "Unable to save the uploaded file"; + $self->AddError("Unable to save the uploaded file", "File"); return undef; } } else { delete $self->{FileName}; - $self->{ErrField} = "File"; - $self->{ErrMessage} = "Unable to upload the file"; + $self->AddError("Unable to upload the file", "File"); return undef; } return 1; @@ -102,8 +100,7 @@ sub _GetFileType($) my $Fh; if (!sysopen($Fh, $self->_GetStagingFilePath(), O_RDONLY)) { - $self->{ErrField} = "FileName"; - $self->{ErrMessage} = "Unable to open '$self->{FileName}'"; + $self->AddError("Unable to open '$self->{FileName}'", "FileName"); return undef; }
@@ -153,8 +150,7 @@ sub _GetFileType($)
if ($self->{FileType} !~ /^(?:exe32|exe64|patch)$/) { - $self->{ErrField} = "FileName"; - $self->{ErrMessage} = "Unsupported file type"; + $self->SerError("FileName", "Unsupported file type"); return undef; }
@@ -169,14 +165,12 @@ sub _AnalyzePatch($)
if (!$self->{Impacts}->{TestBuild}) { - $self->{ErrField} = "FileName"; - $self->{ErrMessage} = "'$self->{FileName}' is not a valid patch"; + $self->AddError("'$self->{FileName}' is not a valid patch", "FileName"); return undef; } if ($self->{Impacts}->{PatchedModuleOrTests} == 0) { - $self->{ErrField} = "FileName"; - $self->{ErrMessage} = "The patch does not directly impact the Wine dlls and programs"; + $self->AddError("The patch does not directly impact the Wine dlls and programs", "FileName"); return undef; } return 1; @@ -194,23 +188,20 @@ sub _ValidateFileName($) if (!defined $self->{FileName}) { delete $self->{FileName}; - $self->{ErrField} = "FileName"; - $self->{ErrMessage} = "You must provide a file to test"; + $self->AddError("You must provide a file to test", "FileName"); return undef; } if (!IsValidFileName($self->{FileName})) { delete $self->{FileName}; - $self->{ErrField} = "FileName"; - $self->{ErrMessage} = "The filename contains invalid characters"; + $self->AddError("The filename contains invalid characters", "FileName"); return undef; } my $PropertyDescriptor = CreateSteps()->GetPropertyDescriptorByName("FileName"); if ($PropertyDescriptor->GetMaxLength() - 32 - 1 < length($self->{FileName})) { delete $self->{FileName}; - $self->{ErrField} = "FileName"; - $self->{ErrMessage} = "The filename is too long"; + $self->AddError("The filename is too long", "FileName"); return undef; }
@@ -290,7 +281,7 @@ sub _ValidateVMSelection($;$)
if (@Deselected) { - $self->{ErrMessage} = "The following VMs are incompatible and have been deselected: @Deselected"; + $self->AddError("The following VMs are incompatible and have been deselected: @Deselected"); return undef; } return 1; @@ -337,8 +328,7 @@ sub _ValidateTestExecutable($;$)
if (!defined $self->{TestExecutable}) { - $self->{ErrField} = "TestExecutable"; - $self->{ErrMessage} = "You must specify the test executable filename"; + $self->AddError("You must specify the test executable filename", "TestExecutable"); return undef; } elsif (!$self->{TestExecutables}->{$self->{TestExecutable}} or @@ -346,8 +336,7 @@ sub _ValidateTestExecutable($;$) # to defend against malicious patches. !IsValidFileName($self->{TestExecutable})) { - $self->{ErrField} = "TestExecutable"; - $self->{ErrMessage} = "Invalid test executable filename"; + $self->AddError("Invalid test executable filename", "TestExecutable"); return undef; }
@@ -393,14 +382,12 @@ sub Validate($) { # Note that Page 1 should have provided a default # so there is no reason for it to still be undefined. - $self->{ErrField} = "Branch"; - $self->{ErrMessage} = "You must specify the branch to test"; + $self->AddError("You must specify the branch to test", "Branch"); return undef; } if (!CreateBranches()->GetItem($self->{Branch})) { - $self->{ErrField} = "Branch"; - $self->{ErrMessage} = "The '$self->{Branch}' branch does not exist"; + $self->AddError("The '$self->{Branch}' branch does not exist", "Branch"); return undef; } } @@ -410,7 +397,7 @@ sub Validate($) return undef if (!$self->_ValidateVMSelection()); if (!$self->{CheckedVMCount}) { - $self->{ErrMessage} = "Select at least one VM"; + $self->AddError("Select at least one VM"); return undef; } } @@ -424,18 +411,17 @@ sub Validate($) !$self->{Run64} and $self->{ShowRun64}) { # Don't set ErrField since Run32 and Run64 may be hidden - $self->{ErrMessage} = "You must at least pick one of the 32 or 64 bit tests to run on the selected Windows VMs."; + $self->AddError("You must at least pick one of the 32 or 64 bit tests to run on the selected Windows VMs."); return undef; }
if ($self->{CmdLineArg} eq "" and !$self->{NoCmdLineArgWarn}) { - $self->{ErrMessage} = "You did not specify a command line argument. ". - "This is most likely not correct, so please ". - "fix this. If you are sure that you really don't ". - 'want a command line argument, press "Submit" '. - "again."; - $self->{ErrField} = "CmdLineArg"; + $self->AddError("You did not specify a command line argument. ". + "This is most likely not correct, so please ". + "fix this. If you are sure that you really don't ". + 'want a command line argument, press "Submit" again.', + "CmdLineArg"); $self->{NoCmdLineArgWarn} = 1; return undef; } @@ -969,8 +955,7 @@ sub OnUnset($) if (!$self->_ValidateFileName()) { # Ignore the error. What counts is not using a suspicious FileName. - delete $self->{ErrField}; - delete $self->{ErrMessage}; + $self->ResetErrors(); } elsif (defined $self->{FileName}) { @@ -1012,12 +997,8 @@ sub OnSinglePage($) my ($self) = @_;
# The checks and defaults are the same as for Page1Next() - if (!$self->OnPage1Next()) - { - # But ignore all errors. The user can change everything anyway. - delete $self->{ErrField}; - delete $self->{ErrMessage}; - } + # But ignore all errors. The user can change everything anyway. + $self->ResetErrors() if (!$self->OnPage1Next());
$self->_SetPage(4); return 1; @@ -1151,7 +1132,7 @@ sub _SubmitJob($$) } if (defined $ErrMessage) { - $self->{ErrMessage} = $ErrMessage; + $self->AddError($ErrMessage); return !1; } } @@ -1210,7 +1191,7 @@ sub _SubmitJob($$) my ($ErrMessage, $Missions) = ParseMissionStatement($WineMissions{$VM}); if (defined $ErrMessage) { - $self->{ErrMessage} = $ErrMessage; + $self->AddError($ErrMessage); return !1; } my $Missions = $Missions->[0]; @@ -1229,34 +1210,32 @@ sub _SubmitJob($$) }
# Now save it all (or whatever's left to save) - my ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save(); + my ($_ErrKey, $_ErrProperty, $ErrMessage) = $Jobs->Save(); if (defined $ErrMessage) { - $self->{ErrMessage} = $ErrMessage; + $self->AddError($ErrMessage); return undef; }
# Stage the test patch/executable so the job can pick it up if (!rename($Staging, "$DataDir/staging/job". $NewJob->Id ."_$self->{FileName}")) { - $self->{ErrMessage} = "Could not stage '$self->{FileName}': $!\n"; + $self->AddError("Could not stage '$self->{FileName}': $!"); return undef; }
# Switch Status to staging to indicate we are done setting up the job $NewJob->Status("staging"); - ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save(); + ($_ErrKey, $_ErrProperty, $ErrMessage) = $Jobs->Save(); if (defined $ErrMessage) { - $self->{ErrMessage} = $ErrMessage; + $self->AddError($ErrMessage); return undef; }
# Notify engine - my $ErrMessage = RescheduleJobs(); - if (defined $ErrMessage) + if ($self->AddError(RescheduleJobs())) { - $self->{ErrMessage} = $ErrMessage; $self->_SetPage(0); $self->{JobKey} = $NewJob->GetKey(); return undef; @@ -1277,15 +1256,15 @@ sub OnSubmit($) my $Staging = CreateNewLink($OldStaging, "$DataDir/staging", "-websubmit2_$self->{FileName}"); if (!defined $Staging) { - $self->{ErrMessage} = "Could not rename '$self->{FileName}': $!"; + $self->AddError("Could not rename '$self->{FileName}': $!"); return undef; } if (!unlink $OldStaging) { unlink $Staging; - $self->{ErrMessage} = $!{ENOENT} ? + $self->AddError($!{ENOENT} ? "$self->{FileName} has already been submitted or has expired" : - "Could not remove the staging '$self->{FileName}' file: $!"; + "Could not remove the staging '$self->{FileName}' file: $!"); return undef; }
@@ -1303,13 +1282,8 @@ sub OnSetShowAllVMs($$) my ($self, $Value) = @_;
# Call _ValidateVMSelection() to identify incompatible VMs so they are not - # marked as Checked - if (!$self->_ValidateVMSelection("deselect")) - { - # Ignore errors - delete $self->{ErrField}; - delete $self->{ErrMessage}; - } + # marked as Checked but ignore errors. + $self->ResetErrors() if (!$self->_ValidateVMSelection("deselect")); $self->{ShowAll} = $Value;
return undef;