On Wed, Aug 16, 2017 at 4:42 PM, Eric Sandeen <sandeen@xxxxxxxxxxx> wrote: > On 8/16/17 4:11 AM, Jan Tulak wrote: >> On Wed, Aug 16, 2017 at 1:07 AM, Eric Sandeen <sandeen@xxxxxxxxxxx> wrote: >>> On 8/15/17 10:08 AM, Jan Tulak wrote: >>>> Save exactly what the user gave us for every option. This way, we will >>>> never lose the information if we need it to print back an issue. >>>> (Just add the infrastructure now, used in the next patches.) >>>> >>>> Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx> >>> >>> >>>> + return NULL; >>>> + } >>> >>> Again, I don't see that any subsequent patches ever call get_conf_raw(); >>> is there any reason to even have this wrapper now? >> >> Luis had a pretty strong opinion about having them there when he will >> need them in his own patches later on and I saw no reason why not to >> add them right now when the functions are created. After all, they >> will be useful later on also for me, when I want to limit the amount >> of asserts and be more return-oriented. It's just not in this >> patchset. > > Hm, ok. Let me reread that, thread - I'm just looking at this patchset > as sent. > > In general, I'm not clear on what your "_safe" and "_unsafe" variants > are about but maybe it was discussed in the prior thread I skipped. > It is about being able to decide what should happen when an error occurs and how good practice it is to just call exit() instead of propagating the error upwards in the stack. Right now, we can abort without a big issue (at least technically), but, for example, with a config file parsing, we will want to print the line of the config file which is invalid. These two threads should give you the details: https://www.spinics.net/lists/linux-xfs/msg08635.html https://www.spinics.net/lists/linux-xfs/msg08636.html In any case, I will wait with sending further versions of these patches until you had the time to look at it. Cheers, Jan -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
