in reply to passing variables between subroutines

Two things. First, I'm not sure why you're validating the remote address. In this chunk of code, there are a number of minor issues, and one larger issue. We'll start with a minor issue: using $ENV{REMOTE_ADDRESS} instead of CGI::remote_host(). It returns what you want, and, should some future CGI standard change how this gets passed around (say under a future mod_perl), you won't need to change anything to work with it - you'll just need to upgrade CGI.pm.

The major issue is the return keyword you're using. I'm not sure what you are attempting to do here. The return keyword is only useful inside a sub, and this doesn't seem to be inside a sub.

To answer your question, change &mailoff to mailoff($info2) (if that's the variable you're trying to pass). And then inside your mailoff sub, do this: 

sub mailoff {
  my $ip_address = shift;
  #open mail handler stuff
  print MAIL "here is the ip address $ip_address";
  #close
}

[download]
Note that the variable name we use inside mailoff is completely unrelated to the variable name we use to call mailoff with. They can be the same name, but I changed it to show that they commonly are different.

Replies are listed 'Best First'.
Re^2: passing variables between subroutines
by budreaux (Initiate) on Jan 16, 2006 at 16:19 UTC
    Tanktalus, Thanks for your reply. I do appreciate the help. I am very green to perl and "perl speak". The reason I am validating an environment variable is the fact that the value could be passed through the url (if someone happened to guess the variable name). From everything I read, it is suggested that "security through obscurity" is wrong. I just want to make sure that my script will be as "bullet proof" as possible. As for the CGI::remote_host(), I'll have to look into that.

    My script was basically pieced together from other scripts I had found. I tried the best I could to understand what was going on and thus the reason for the incorrect use of "return".

    I think I understand what you are saying, and I will give this a try.
[reply]

      Sorry, I completely misread your original code. I see what the return is doing now. It's inside another sub. I just didn't see the sub because of your indenting. While it's true that whitespace (mostly) doesn't matter to the computer, the fact is that code is not written for the computer as much as it is written for the next human that has to read it (usually yourself). You should check out perlstyle for some ideas on how to improve the layout of your code. For example, your IP_check sub could look better this way: 

      sub IP_check {
    my $_info2 = $ENV{'REMOTE_ADDR'};
    my $info;
    
    if (!$_info2) {
        $info2 = "No IP address available";
        return $info2;
    } elsif ($_info2 !~ /[\%]+/ && 
             $_info2 =~ /^(\d{1,3}\.{1})(\d{1,3}\.{1})(\d{1,3}\.{1})(\
+d{1,3})$/) {
        my $info2 = "$1$2$3$4";
        return $info2;
    } else {
        my $info2 = "Bad IP address submitted";
        return $info2;
    }
}

      [download]
      Notice how I did nothing but add whitespace. It's much easier to follow now. Now that I can read it properly, the return is perfectly legal. It may not be completely optimal, but it's legal.

      Instead, if I can just focus on this sub for a minute, you may want to use the $info you declared rather than the global $info2 you are using. That helps keep the subroutine from having side effects - side effects aren't necessarily bad, but when you can avoid them, do so, as it will make your code easier for the human to follow.

      Or, to make the subroutine just a bit tighter, you don't need that at all. For example: 

      sub IP_check {
    my $_info2 = $ENV{'REMOTE_ADDR'};

    if (!$_info2) {
        return "No IP address available";
    } elsif ($_info2 !~ /[\%]+/ && 
             $_info2 =~ /^(\d{1,3}\.{1})(\d{1,3}\.{1})(\d{1,3}\.{1})(\
+d{1,3})$/) {
        return "$1$2$3$4";
    } else {
        return "Bad IP address submitted";
    }
}

      [download]
      Now, some people get mad at having multiple returns in a single subroutine, arguing that it's easier to follow when there's a single entry and a single exit to each function. I'm not a complete believer in this style, but I'm not going to say it's wrong, either. So here's that example: 
      sub IP_check {
    my $_info2 = $ENV{'REMOTE_ADDR'};
    my $ip;

    if (!$_info2) {
        $ip = "No IP address available";
    } elsif ($_info2 !~ /[\%]+/ && 
             $_info2 =~ /^(\d{1,3}\.{1})(\d{1,3}\.{1})(\d{1,3}\.{1})(\
+d{1,3})$/) {
        $ip = "$1$2$3$4";
    } else {
        $ip = "Bad IP address submitted";
    }

    return $ip;
}

      [download]
      I'll leave it up to you to decide which you think is more readable and easier to understand. Then there's the more perlish way to do all of the above - but it's a wee bit tricky, I'd suggest one of the above over this next example. This next one only works because the if statement is the last statement in the subroutine since, in perl, the return value from a sub is determined from the last value in the subroutine if no 'return' is otherwise encountered. (It's because it requires a long explanation that I don't recommend it ;-}) 
      sub IP_check {
    my $_info2 = $ENV{'REMOTE_ADDR'};
    my $ip;

    if (!$_info2) {
        "No IP address available";
    } elsif ($_info2 !~ /[\%]+/ && 
             $_info2 =~ /^(\d{1,3}\.{1})(\d{1,3}\.{1})(\d{1,3}\.{1})(\
+d{1,3})$/) {
        "$1$2$3$4";
    } else {
        "Bad IP address submitted";
    }
}

      [download]
      Now the next question is ... how to use it? That's easy - regardless of which one of the above you use, you call it like this: my $info2 = IP_check(); Hope that helps!

[reply]
[d/l]
[select]

In Section Seekers of Perl Wisdom