Dne 7.10.2011 11:16, jsuchome@svn2.opensuse.org napsal(a):

-string SnapperAgent::getValue (const YCPMap map, const string key) +string SnapperAgent::getValue (const YCPMap map, const string key, string deflt) { if (!map->value(YCPString(key)).isNull() && map->value(YCPString(key))->isString()) return map->value(YCPString(key))->asString()->value(); else - return ""; + return deflt; }

The "key" parameter is not changed so you could pass a reference to it ("const string &key"), that will prevent from creating unnecessary copy of the string on function call. "deflt" is also not changed, passing "const string &deflt" would help (I'm not sure, I'm not C++ expert, but then you might need to create the copy explicitly via return string(deflt); at the end.)

@@ -252,13 +282,10 @@ /** * Return the given file mode as octal number */ -global integer GetFileMode (string file) { +integer GetFileMode (string file) {

map out = (map) SCR::Execute (.target.bash_output, "/bin/stat --printf=%a " + file);

When building custom shell commands String::Quote() should be always used, it prevents from errors when the input contains spaces and improves security (command injection) if the input comes from user.

if (Popup::AnyQuestionRichText (_("Restoring files"), // popup message, %1 is snapshot number, %2 list of files sformat (_("These files will be copied from snapshot '%1' to current system: <p>%2</p>Are you sure?"), - previous_num, mergestring (files, "<br>")), + previous_num, mergestring (to_restore, "<br>")),

Similar problem, although very unlikely, if a file name contains an HTML tag it would be wrongly displayed. Use String::EscapeTags().

- SCR::Execute (.target.bash, sformat ("/bin/cp -a '%1' '%2'", orig, file)); + SCR::Execute (.target.bash, sformat ("/bin/cp -a '%1' '%2'", orig, full_path));

String::Quote() should be also used here, '%1' helps when there is a space, but doesn't help when the string contains ' (a single quote). And if the command supports (cp does) we should use -- option to stop the option processing. If %1 begins with -- it would be processed as an option, in some cases this can completely change the meaning of the command. The most fool proof way is to execute shell commands like this: SCR::Execute (.target.bash, sformat ("/bin/cp -a -- '%1' '%2'", String::Quote(orig), String::Quote(full_path))); (Note: These problems are not critical in YaST because YaST is started by root, but it's more critical in webyast and therefore we should get used to think about these possible problems.) -- Ladislav Slezák Appliance department / YaST Developer Lihovarská 1060/12 190 00 Prague 9 / Czech Republic tel: +420 284 028 960 lslezak@suse.com SUSE -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org