Code review comment for lp:~evfool/synaptic/lp945997

Revision history for this message
Daniel Hartwig (wigs) wrote :

The comment “try multiarch name next” no longer applies, replaced by the new comment you insert.

The logic here seems ok.

Portability issues:

+ int index = namestr.find_first_of(":");
+ if (index > -1) {

string::find_first_of returns string::size_type, not int. It should not be coerced prior to comparing it to the “char not found“ value, or using it with any string functions. Generally, it is never necessary to explicitly coerce string::size_type.

The “char not found” value is string::npos, not -1. Although equivalent for some implementations of string in some libraries, you can not rely on that:

string::size_type index = namestr.find_first_of(":");
if (index != string::npos) {

review: Needs Fixing

« Back to merge proposal