Heiko Voigt <hvoigt@xxxxxxxxxx> writes: > In case the machine has only one cpu the initialization was > skipped. An obvious question that forces reviewers to go "Huh?" is why this matters. If use_threads is false, we do not call start_threads(), and I think at least the intent of the threaded grep code in 5b594f4 (Threaded grep, 2010-01-25) is that uninitialized mutexes matter at all in that case. For example, "grep_mutex" is touched by calling grep_lock() and grep_unlock() macros, or pthread_* functions on it directly. There are five functions that touch this mutex: add_work(), get_work(), work_done(), start_threads(), wait_all(). - add_work() is called from grep_sha1_async() and grep_file_async(). They are both called _only_ when use_threads is true. - get_work() and work_done() are only called from run() and that is the function threads run from start_threads(). They cannot be reached unless use_threads is true. - wait_all() has three call sites in cmd_grep(); all of them trigger only when use_threads is true. So at least for grep_mutex your patch shouldn't matter. Please explain use of which mutex is broken and how your patch fixes it. I think the fix also assumes that an initialized mutex that is used but not destroyed is not a programming error, as wait_all() is the ONLY function that destroys these mutexes and it is called ONLY when we are using threads by calling start_threads(). But that assumption surely smells like it is sweeping a bug under a rug rather than fixing it. Is there some programming rule I am not aware of, like "if you define a mutex or cond, you must initialize it, even if you are not going to use it at all?" If that is the case, I think your patch is necessary, but I somehow find it a bit hard to believe. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html