On Thu, Oct 25, 2012 at 5:46 PM, Ming Lei <ming....@canonical.com> wrote: > struct file *file; > - snprintf(path, PATH_MAX, "%s/%s", fw_path[i], buf->fw_id); > + > + if (i < 0) { > + if (!fw_path_para[0]) /* No customized path */ > + continue; > + snprintf(path, PATH_MAX, "%s/%s", fw_path_para, > + buf->fw_id); > + } else { > + snprintf(path, PATH_MAX, "%s/%s", fw_path[i], > + buf->fw_id); > + }
Ugh. This is just disgusting. Please just make "fw_path[0]" just be the pointer to fw_path_para[] (which sounds like the cleanest fix) and get rid of the negative 'i' and conditional entirely. Or if there is some odd reason you don't want to do that, at least make the conditional much smaller, without the snprintf() in both arms (ie make the if-statement just set a "const char *dir" variable to either fw_path[i] or fw_path_para or whatever). Sure, the compiler *may* merge them (gcc does, but I've seen it miss them too), but even if the compiler might fix up ugly code, that's not a reason for it to be ugly in the source code anyway. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/