On Tue, Feb 26, 2013 at 02:46:30PM -0600, Eric Sandeen wrote: > On 2/26/13 2:40 PM, Ilya Dryomov wrote: > > On Tue, Feb 26, 2013 at 02:20:30PM -0600, Eric Sandeen wrote: > >> The coverity runs had a false positive complaining that save_ptr > >> is uninitialized in the call to strtok_r. > >> > >> We could initialize it, but Zach points out that just using > >> strsep is a lot simpler if there's only one delimiter, > >> so just switch to that. > >> > >> Signed-off-by: Eric Sandeen <sand...@redhat.com> > >> --- > >> > >> V2: Remove accidentally-added debug printfs, thanks Geoffredo! > >> > >> diff --git a/cmds-balance.c b/cmds-balance.c > >> index b671e1d..cfbb8eb 100644 > >> --- a/cmds-balance.c > >> +++ b/cmds-balance.c > >> @@ -67,11 +67,8 @@ static int parse_one_profile(const char *profile, u64 > >> *flags) > >> static int parse_profiles(char *profiles, u64 *flags) > >> { > >> char *this_char; > >> - char *save_ptr; > >> > >> - for (this_char = strtok_r(profiles, "|", &save_ptr); > >> - this_char != NULL; > >> - this_char = strtok_r(NULL, "|", &save_ptr)) { > >> + while ((this_char = strsep(&profiles, "|"))) { > >> if (parse_one_profile(this_char, flags)) > >> return 1; > >> } > >> @@ -136,14 +133,11 @@ static int parse_filters(char *filters, struct > >> btrfs_balance_args *args) > >> { > >> char *this_char; > >> char *value; > >> - char *save_ptr; > >> > >> if (!filters) > >> return 0; > >> > >> - for (this_char = strtok_r(filters, ",", &save_ptr); > >> - this_char != NULL; > >> - this_char = strtok_r(NULL, ",", &save_ptr)) { > >> + while ((this_char = strsep(&filters , ","))) { > > > > ^^^ whitespace > > > > > > One of the differences between strtok() and strsep() is that the former > > allows multiple delimiters between two tokens. With strsep(), this > > > > btrfs balance -dfoo1=bar1,,,foo2=bar2 <mnt> > > > > fails with error, whereas with strtok() it passes. I don't have a > > strong opinion here (this has been loosely modeled on the way mount(8) > > handles -o options), but might it be better to just initialize save_ptr? > > (And yes, I know that strsep() is better ;)) > > I don't really care much either way, TBH. Initializing it seems a little > bit magic, but with a comment as to why, it'd be fine. If you did it this > way intentionally to allow the above format, and changing it would break > expectations, then I'll happily just initialize save_ptr.
Yeah, although I doubt anybody would notice, it's probably better to keep the old behaviour. > > (And, I realize that lots of these changes are pedantic and seemingly > pointless, but we've gotten the static checker errors down from over 100 > to under 30 and dropping; the more noise we remove the more likely we are > to pay attention to the output and catch actual errors. At least that's > my feeling; if people think this is getting to be pointless churn, I'm > ok with that, too). Not at all. Btrfs-progs had rarely seen any cleanups, with you and Zach patching it up and David's integration work it has gotten a whole new life ;) BTW, huge thanks for the kernel-style build output, I am forever grateful.. Thanks, Ilya -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html