On Fri, 29 Jul 2011, Christophe CURIS wrote: hi,
> Please find attached a new patch (replacing my previous) with Tamas' > feedback. this looks much better, thanks. > > > --- a/WINGs/proplist.c > > > +++ b/WINGs/proplist.c > > > @@ -1677,7 +1677,7 @@ int wmkdirhier(const char *path) > > > while (p++ < plen && thePath[p] != '/') > > > ; > > > > > > - strncpy(buf, thePath, p); > > > + strncpy(buf, thePath, p); // TODO: This is not great. > > No guarranty > > > that p < sizeof(buf) > > > > are you sure about this? i've been tryng to get back in the mindset i > > was in when i wrote this (not much success, though), but i don't see > > where or how can this overflow. can you describe a situation when it > > does? > > I will re-look at this to see what I had in mind when I added the comment > and > let you know. (Although if you don't see anything coming, that may just > mean > it was too late in the night when I first added the comment ;-) ) not necessarily. the fact that even i (who has written it originally) couldn't decipher this fragment tells quite a lot about its quality (the lack of it, that is). > > > --- a/src/defaults.c > > > +++ b/src/defaults.c > > > @@ -2006,6 +2006,7 @@ static int getKeybind(WScreen * scr, WDefaultEntry > > > * entry, WMPropList * value, } > > > > > > strncpy(buf, val, MAX_SHORTCUT_LENGTH); > > > + buf[MAX_SHORTCUT_LENGTH-1] = '\0'; > > > > please don't. convert src/ to use wstrlcpy (and wstrlcat, where > > applicable). > > Sounds so sane now that you can see it in the new patch! this new patch looks good. you could ponder a bit about what to do on truncation, but i'm leaning towards being silent here is fine (maybe log a warning or whatnot). carlos, do commit this if you see fit. -- [-] mkdir /nonexistent -- To unsubscribe, send mail to [email protected].
