Laurent_R
Enthusiast
Dec 28, 2012, 2:59 PM
Post #2 of 3
(514 views)
|
|
Re: [mpride] Net::FTP - Returing 'Bad remote filename" error
[In reply to]
|
Can't Post
|
|
Please don't kill me for lack of comments and indentation etc.
Why should we want to kill you for that? No point, really. You are shooting the bullet into your head yourself, you don't need any help for your suicide, you're doing it yourself very properly and very efficiently...
Otherwise, I have not used the Net::FTP module in ages and don't know if your FTP commands are correct, but I can spot a couple of things that seem wrong or suboptimal to me.
The most important point is that the logic of your loop seems to be wrong:
$file = shift(@array);
do
{
use Net::FTP;
$ftp = Net::FTP->new("10.9.80.38");
$ftp->login("$userid", $password)
or die "Cannot login ", $ftp->message;
$ftp->ascii;
$ftp->cwd("/wireless/syslog")
or die "Cannot change working directory ", $ftp->message;
$ftp->put($file);
}
until (@array eq 0);
Basically, you are reading the first filename in your array and then only enter the loop that is supposed to stop once the array has been exhausted, which will of course never happen, since you are not reading the array within the loop (unless your array has only one filename).
The right way to do this would be something like this:
foreach my $filename (@array) {
# do your FTP commands with $filename
# ...
}
A couple of other observations:
- chomp your filenames before you use them when they are obtained from a system command, it is very unlikely to work otherwise;
- put the "use Net::FTP; " statement at the beginning of the script, not within the loop
- the first "{" block opening is useless (and so is the corresponding closing statement)
- don't name your array @array, it is clear it is an array from the sigil (the @ sign at the beginning), give it a name that make sense to your program, such as, say, @list_of_the_names_of_the_files_that_I_want_to_transfer (I am overdoing it somewhat here, but I hope you get the point, @list_of_files would already be much better)
- if you open the FTP connection within the loop, then close it also within the same loop, before opening the next one (but it would probably make more sense to open the FTP connection only once, outside the loop, and handle only the tranfer of each file within the loop, and close the connection at the end).
- there are better ways to list files and remove them that using the "system" command, look at the Perl built-in commands to do that (commands such as "glob", "readdirr, "unlink", etc.).
But, of course, the most serious errors listed above, you probably would have seen immediately if you had taken just a few minutes to indent your code correctly... And you would probably have saved a lot of hours wasted stupidly by usefully spending these few minutes.
;-)
|
MAIN
SEARCH
WHO'S 
LOG 