MyBB Community Forums

Full Version: sprintf/GetSQLValueString
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
Trying to clean up someone's crappy code here, a lot of queries are formatted like this:

$query = sprintf("SELECT * FROM products WHERE pid = %s", GetSQLValueString($prod, "text"));

Is there a reason it isn't just done like this?

$query = "SELECT * FROM products WHERE pid = '$prod'";

All I know about sprintf is it increases your memory usage by a lot, and I've never even heard of getsqlvaluestring.

Also, if there's a better forum I could be asking these, please let me know. (Digital Point is useless)

EDIT: Looks like getsqlvaluestring is a function for escaping (but mysql_real_escape_string is already being used), and I don't understand what the documentation for sprintf is getting at.
From what I can gather sprintf just allows you to format the result into anything you want Confused
This:

echo sprintf("MyBB is the best free %s software", "forum");

Outputs:

MyBB is the best free forum software

Seems totally pointless aside from maybe making the code a little easier to work with in certain situations?...
It's pointless as long as you've already sanitised the $prod variable Toungue
It's the new "trend" to use placeholders in the SQL.

Look at the PDO prepare function for instance. This way you can't use $_POST or $_GET variables directly in your SQL so you minimize the changes to have an SQL injection and also (I can only speak for PDO) it sanitizes your SQL for you.
I can understand the idea behind it. from a coding standpoint it makes is easy to check if the data being passed is being sanitzed since as long as the query has the GetSQLValueString function it should be clean rather than having to find each variable to check if it is sanitized.

also, using sprintf() allows multiple replacements all at once. MyBB uses it a lot with the language variables. it will replace a placeholder in order from left to right with the other items in the function parameters. beasts having to do multiple str_replace() which will only do one placeholder at a time.
(2011-01-16, 05:42 PM)pavemen Wrote: [ -> ]I can understand the idea behind it. from a coding standpoint it makes is easy to check if the data being passed is being sanitzed since as long as the query has the GetSQLValueString function it should be clean rather than having to find each variable to check if it is sanitized.

also, using sprintf() allows multiple replacements all at once. MyBB uses it a lot with the language variables. it will replace a placeholder in order from left to right with the other items in the function parameters. beasts having to do multiple str_replace() which will only do one placeholder at a time.

sprintf and str_replace are used in different cases.

sprintf is used when you want to put a value in a defined place in your string. str_replace is used when you want to replace something, not necessarily a mark (like %s or %d are for sprintf).
(2011-01-16, 06:09 PM)Pirata Nervo Wrote: [ -> ]sprintf and str_replace are used in different cases.

sprintf is used when you want to put a value in a defined place in your string. str_replace is used when you want to replace something, not necessarily a mark (like %s or %d are for sprintf).

I understand that. I was trying to convey the ability to make multiple replacements in one line instead of having to make individual replacements across multiple lines.