On Thu, Jul 19, 2012 at 2:43 PM, Gustavo Sverzut Barbieri < [email protected]> wrote:
> On Thursday, July 19, 2012, Daniel Wagner wrote: > >> Hi Gustavo, >> >> [why are you guys posting html mails?] > > > Blame iOS, most of my reviews are done while waiting someone, from my > phone :-) > > >> >> On 19.07.2012 04:08, Gustavo Sverzut Barbieri wrote: >> >>> This makes sense. I can assume that STORAGEDIR/d->d_name is a dir >>> and just fstatat/stat on "settings". If it fails, this could be >>> that STORAGEDIR/d->d_name is not a dir a dir or there is no settings >>> file inside. So it will "continue". I can patch this. >>> >>> And god, you know the size of path prefix, just join the >>> strings wisely and >>> not sprintf it :-/ >>> >>> >>> I don't get it here. Would you please detail it a little. I don't >>> understand why a string concat would be better than a sprinf... >>> >>> >>> snprintf is stupid, it cant know the size to allocate, the amount to >>> copy... >>> >>> If you know sizeof(STORAGEDIR) and strlen() you can malloc and memcpy(), >>> much faster. >>> >>> If it's a loop, the prefix/directory can be strlen() and memcpy() only >>> once, just changing the variant part (filename) while you do the >>> invariant only once >>> >> >> I see your point, but I rather not have these kind of micro optimization >> at this point. We have plenty of g_string_append_printf in our code base >> and just fixing one place isn't good enough. >> >> Andrei, if you have the version where the two paths are concatenated than >> that is enough. No need to do this error prone strlen() memcpy() stuff >> (remember Ross' fix?)? >> > > > Pfffft! Error prone! > > Okay, fair enough getting it in. The actual problem I had was with 2 stat > while just one is needed > > Back from vacation. I will submit patches again taking into consideration your observations. Thank you, ag _______________________________________________ connman mailing list [email protected] http://lists.connman.net/listinfo/connman
