hukeping <hukeping@xxxxxxxxxx> writes: >>I do not mind getting rid of the "FORMAT_PATCH_NAME_MAX" constant and >>replacing it with a variable that defaults to 64 and can be tweaked by a command >>line option and/or a configuration variable. >>It does not feel it is worth the effort to replace one hardcoded constant with >>another hardcoded constant. >> >>> Looking at the code which uses the constant, I suspect it could also >>> be made simpler: >>> >>> - the PATH_MAX check in open_next_file() seems pointless. Once upon a >>> time it mattered for fitting into a PATH_MAX buffer, but these days >>> we use a dynamic buffer anyway. We are probably better off to just >>> feed the result to the filesystem and see if it complains (since >>> either way we are aborting; I'd feel differently if we adjusted our >>> truncation size) >>> >>> - the logic in fmt_output_subject() could probably be simpler if the >>> constant was "here's how long the subject should be", not "here's >>> how long the whole thing must be". >>> >>> But those are both orthogonal to your patch and can be done separately. >> >>Yes, these clean-ups seem worth doing. > > Agreed, and I'd like to do it with two separated commits: > - commit-1, cleanup the open_next_file() by drop the if (filename.len>=..) statements. > > - commit-2, replace FORMAT_PATCH_NAME_MAX in fmt_output_subject() with a constant > in there and make it to 80(or other value?), and drop FORMAT_PATCH_NAME_MAX > from log-tree.h. > > Is this works for you? I am not sure what you meant by "Agreed". I said two things: - It is dubious that it is worth the effort to replace a hardcoded constant with another. Making it configurable with command line option and/or configuration variable may be worth doing. - Two observations Peff made for further clean-up are probably worth doing. If you are agreeing to both of the above and following through, then yes, it seems like a good plan. If agreement is only to the former, it probably still is worth doing. Anything else, I don't know.
