On Sun, Feb 3, 2019 at 11:00 AM Fabien COELHO <coe...@cri.ensmp.fr> wrote:
> > Hello David, > > >> I do not understand why dump_inserts declaration has left the "flags for > >> options" section. > > > > I moved that because it's no longer just a flag. It now stores an int > value. > > Hmmm. Indeed, all th "int"s of this section should be "bool" instead. Now, > some "flags" do not appear although the culd (clear, createdb, blobs), so > the logic is kinda fuzzy anyway. Do as you wish. > > >> I'd suggest not to rely on "atoi" because it does not check the argument > >> syntax, so basically anything is accepted, eg "1O" is 1; > > > > Seems like it's good enough for --jobs and --compress. Do you think > > those should be changed too? or what's the reason to hold > > --rows-per-insert to a different standard? > > I think that there is a case for avoiding sloppy "good enough" programming > practices:-) Alas, as you point out, "atoi" is widely used. I'm campaining > to avoid adding more of them. There has been some push to actually remove > "atoi" when not appropriate, eg from "libpq". I'd suggest to consider > starting doing the right thing, and left fixing old patterns to another > patch. > > at least for processing user argument i think it is better to use strtol or other function that have better error handling. i can make a patch that change usage of atoi for user argument processing after getting feedback from here or i will do simultaneously regards Surafel