roopurt18

mysql_fetch_array() and mysql_fetch_assoc() both return an array, except that mysql_fetch_assoc() returns an associative array, which is preferred for accessing the data.

Oops. $tmp_uname = ${'uname' . $count}; $tmp_title = ${'name' . $count};

A few items you need to know. First: SELECT id, id You are only going to get a single `id` column back in your result set since one will overwrite the other. You need to alias one of them with something like: SELECT `id`, `id` AS `theAlias` Second: FROM d_media, library_docs This is an implicit INNER JOIN, however do not use this syntax. Use explicit INNER JOINs instead and it also helps to alias the table. You also have to specify which field to join the tables on: FROM `users` u /* u is the alias */ INNER JOIN `user_profiles` p ON u.`id`=p.`user_id` /* specify which fields join the tables */ So your query might look like: SELECT m.`fname` FROM `d_media` m INNER JOIN `library_docs` d ON d.`file_id`=m.`id` WHERE d.`id`={$docid} There are some examples of JOINs in the MySQL documentation, which you should take a look at: http://dev.mysql.com/doc/refman/5.0/en/join.html

Your algorithm for displaying the manufacturers and their models is very inefficient. Let M equal the number of manufacturers. Let N equal the number of models. Outer loop iterates M times Inner loop iterates N times Your code is performing M * N iterations, which is fine for small sets of data, but you've already stated you have 3000+ models. This means even with just two manufacturers you are performing 6000+ iterations, which is a lot of looping. Even worse is the possibility you are not doing any work during these iterations, since you have this statement: if($row2['manu_id']==$row1['manu_id']) So let's say that for our two manufacturers, the first has 3 products and the second has 5 products. Your code will still perform 6000+ loops. This is very, very bad. Enter the INNER JOIN to make our life easier. Ok...Scratch that thought on the INNER JOIN. INNER JOIN would be appropriate in the instance where you are selecting from two different datasets or tables. Copying your code into my editor though, I see that you are running both queries against the same table. So we will not be using INNER JOIN to solve your problem, here is your improved code: <?php // We want all of the models $sql = " SELECT `id`, `manu_id`, `modal_no` FROM `new_product` WHERE `modal_no`<>'' ORDER BY `manu_id` "; // Run our query $q = mysql_query($sql); // Check for success if(!$q){ echo "Error: Unable to pull models."; exit(); } // Here we loop over our models. Each time we encounter a new manufacturer, // we also want to insert an html header for that manufacturer. For that we // use the $curManu variable $curManu = null; // No manufacturer, yet while($row = mysql_fetch_assoc($q)){ // Check if we are encountering a new manufacturer if($row['manu_id'] !== $curManu){ // We have a new manufacturer // Before we can echo, we must close our previous manufacturer if($curManu !== null){ // We only had a previous manu. if this var != null echo '</p>'; } echo "<p><strong>{$row['manu_id']}</strong><br />"; $curManu = $row['manu_id']; } echo "<a href='./detail.php?prod_id={$row['id']}'> {$row['modal_no']}</a> "; } // When we exit the loop will will have one more unclosed manu. echo '</p>'; ?>

Change: $query1 = "Update vp_data SET uname = '$uname".$count."' WHERE title='$name".$count."'"; to $tmp_uname = {'uname' . $count}; $tmp_title = {'name' . $count}; $query1 = "UPDATE `vp_data` SET `uname` = '{$tmp_uname}' WHERE title={$tmp_title}";

Javascript is not a reliable solution because users can turn it off.

SELECT * FROM users WHERE User = '$user' AND Password = '$pass' If you have a lot of users in your DB, create an index on `User` and maybe the first 5 or 6 characters of `Password`. That should help the select query run quickly. update users set online='<img src=\"/images/online.png\" border=\"0\" />' WHERE User = '$user' The index above will also help with the speed of this query. But why on Earth are you storing an image path in the database? Change your `online` field to a TINYINT and set it to 1 if the user is online and 0 if they are not. Alternatively you can use an ENUM field. Then you use your program logic to display the online.png graphic for that user if they DB says they are online. If you have relatively few users in your database, then it might be taking ages due to numerous factors. A heavy server load, slow internet connection, something else inefficient in your program, or a million and one other possibilities.

If you need more coffee then likely you need more sleep. Cure the disease, not the symptoms.

You're setting $num to zero each time you enter the loop.

Takes ages? It shouldn't. What is the query you're running?

$user = mysql_real_escape_string(stripslashes(htmlentities(ucwords(strtolower($_POST["userid"]))))); $pass = md5(mysql_real_escape_string(stripslashes(htmlentities($_POST["password"])))); How are you using these after the assignments? Have you tried echo'ing the query you're generating and checking that it is correct?

I don't know, maybe with: $variable = split(",",trim($ff['FORMFIELDNAME1'] . $ff['FORMFIELDNAME2'])); It's hard to say because you haven't been very specific with what you're trying to accomplish here.

There is a very high chance your use of htmlentities() is incorrect. Take a look at this thread for the explanations why: http://www.phpfreaks.com/forums/index.php/topic,185847.0.html

Why is it you can not change the name of either the textarea or the checkbox controls? Because you're going to have to.

^ In addition, there is also the case of reporting. If you are pulling the data for display in a container that does not render HTML, such as a reporting application, then you are going to wind up with all sorts of junk if you called something like htmlentities() before storing in the database. Or say you later want to write a cron job that will search your database for attempts by users to insert malicious code. Now you will have to search through all sorts of < or > instead of simple < or >.

See my replies to another post about the same thing: http://www.phpfreaks.com/forums/index.php/topic,185869.msg832280.html#msg832280 http://www.phpfreaks.com/forums/index.php/topic,185869.msg832312.html#msg832312

Give us the stripped down HTML for your form as it was when you originally posted. Also give us the format you want the data to be in $_POST. Why?

The power of Google: http://www.php.net/manual/en/function.get-magic-quotes-gpc.php

This error would indicate that the query did not run successfully, which means it was probably invalid. Try echo'ing the query to the screen and examining it to make sure it is a syntactically correct MySQL statement.

No. If your site is only cleaning the data before it goes into the database and I can perform MySQL injection on your site, then I can place unclean data directly into your database. Then when you display my unclean data your code will not clean it before display because you've made the assumption that everything in your database is clean, which it no longer is.

Possibly: WHERE ( `userid`='$userid' OR ( `username`='$username' AND `password`='$password' ) AND `banned`=0

This is true, but I recommend against it. The reason is you now have data duplication. If you store the user's email in the session and the user updates their profile, you now have to remember to update it in the session or anywhere else you've temporarily stored it. The more you duplicate data in this manner the more likely you are to forget to update the data and introduce a bug in your program. By storing just the user's id you can query any information for the user either when the page loads or just on pages that use it. This is the practice I use and I don't find that it creates any performance issues while it does make my code easier to maintain and less cumbersome.

@agricola I've seen quite a few pieces of source code that do the same thing. In the end, and this is my opinion, they appear cluttered and no easier to read or maintain. My recommendation is that if your code blocks are getting so big you find that necessary, then it is time to take the body and put it in a function. The following example is self-documenting and clean. <?php // Validate all of our items while($item = getSomeItemFromAList()){ if(isValidItem($item)){ processItem($item); }else{ promptUserToFixItem($item); } } ?> The following is not: <?php // Validate all of our items while($item = getSomeItemFromAList()){ if(isValidItem($item)){ // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! }else{ // ITEM IS INVALID // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! } // END ITEM VALID CHECK } // end item loop ?>

What I'm saying is you don't need logged. The only time you set those is if the user is logged in, right? So if userid is set then they are logged in, making the logged value unnecessary.

Do not store their password in the session. The only item you need in the session to keep track of which user it is and if they're logged in is the user_id.