0

my problem is that when I use mysql in global scope it works, but inside it isn't, have a look at code please:

//connect.php
@mysql_connect($mysql_server, $mysql_admin, $mysql_pass);
@mysql_select_db($mysql_db);

//main.php
require_once("connect.php");
$rReq = $_REQUEST["req"];

function failed()
{
        $qe = mysql_query("SELECT success_count FROM db WHERE serial='".$rReq."'");
        $ro = mysql_fetch_row($qe);
        $ro[0]+=1;
        mysql_query("UPDATE db SET success_count = '".$ro[0]."' WHERE serial='".$rReq."'");
}

//main code
failed(); // not works, mysql_query does nothing

//if i put here the same code but outside the function it works :/
$qe = mysql_query("SELECT success_count FROM db WHERE serial='".$rReq."'");
$ro = mysql_fetch_row($qe);
$ro[0]+=1;
mysql_query("UPDATE db SET success_count = '".$ro[0]."' WHERE serial='".$rReq."'");
psychoboi111
  • 108
  • 6
  • You should declare global variables with `global` keyword inside the functions. Without this the variables, such as $rReq, are local variables (with empty value). – Stan Dec 25 '12 at 16:58

3 Answers3

1

The $rReq variable is not defined inside the function scope, that's why it fails. You should pass it as a parameter to the function, if possible.

Also I recommend not to suppress any errors, ever. It is considered bad practice and if you would have had error reporting on, a "Undefined variable..." error would have helped you out.

Shoe
  • 74,840
  • 36
  • 166
  • 272
1

You are not doing any error checking in your query, so it's no wonder it fails without telling you what goes wrong. That should be the first thing to fix. See the manual or this question on how to add proper error checking. Also, the @ will suppress error messages when connecting - remove them and you'll get information on what goes wrong.

As you say, your concrete problem is to do with scope. You'd need to pass all the variables you need to the function, for example like so:

function failed($rReq, $qe)
  { 
   ....
  }

Also, note that your code is vulnerable to SQL injection, which is very dangerous.

The mySQL extension is outdated and it's not a great idea to start writing new code with it. Consider switching to PDO instead - I know it's an additional strain when learning new stuff, but it's really worth the additional effort!

Community
  • 1
  • 1
Pekka
  • 442,112
  • 142
  • 972
  • 1,088
  • thanks, now I understand :) I used $GLOBAL[]. It's not my whole code, I do antisql things. Yea, it's my fault cuz my server turned off displaying errors – psychoboi111 Dec 25 '12 at 17:11
0

In order for the $rReq variable to be visible inside the function, you need to declare it there, pass it in as an argument, or use global to pull it in:

function failed()
{
    global $rReq;
    $qe = mysql_query("SELECT success_count FROM db WHERE serial='".$rReq."'");
    ...

Variables from outside a function are not automatically visible within it, and variables from inside a function are not visible outside of it.

Amber
  • 507,862
  • 82
  • 626
  • 550
  • 1
    While technically this answers the question at hand, you should also mention 1) SQL injection, 2) mysql_ deprecation, and probably 3) the fact that the two queries can be folded into one. – DCoder Dec 25 '12 at 17:11
  • There are far more issues than those 3 with the code above; this isn't codereview.stackexchange.com – Amber Dec 25 '12 at 17:12