Purpose of the struct is that if I forgot to close some FD it would get automagically closed once the object is destroyed (it's created on stack, so once it get removed from stack it get auto-closed)
your change is correct, I should have make sure that closing it multiple times doesn't cause troubles, however I don't see where I am closing it three times? On Sun, Jun 23, 2013 at 12:25 AM, Platonides <[email protected]> wrote: > On 22/06/13 20:24, Petr Bena wrote: >> >> resolved. > > > No, you're still vulnerable (1609de5f). > Moreover, your are changing a file different than the one you checked. > > I also noticed that you are closing each descriptor thrice. > > One instance is trivially fixed by: >> >> diff --git a/include/Take.h b/include/Take.h >> index dd063f3..4fa2a41 100644 >> --- a/include/Take.h >> +++ b/include/Take.h >> @@ -20,7 +20,7 @@ class Take >> int fd; >> FD(int f): fd(f) { }; >> ~FD() { Close(); }; >> - void Close() { if(fd >= 0) close(fd); }; >> + void Close() { if(fd >= 0) close(fd); fd = -1; >> }; >> operator int (void) const { return fd; }; >> }; > > > > The other close was hidden in copied objects (why did you need an object? > This would have been much easier simply passing the fds!) > >> diff --git a/include/Take.h b/include/Take.h >> index dd063f3..ac8a417 100644 >> --- a/include/Take.h >> +++ b/include/Take.h >> @@ -29,10 +29,10 @@ class Take >> virtual ~Take(); >> protected: >> private: >> - bool Overtake(string path, FD fd); >> + bool Overtake(string path, FD& fd); >> bool Verify(string path); >> static int Callback(const char* path, const struct stat *sb, int >> typeflag, struct FTW *ftwbuf); >> - static void ChangeOwner(string path, uid_t owner, FD fd, bool >> ChangeGroup); >> + static void ChangeOwner(string path, uid_t owner, FD& fd, bool >> ChangeGroup); >> static bool CheckGroups(struct stat info); >> static bool CheckHL(string path); >> static bool CheckHL(struct stat info); >> diff --git a/src/Take.cpp b/src/Take.cpp >> index 8a111d6..accfc02 100644 >> --- a/src/Take.cpp >> +++ b/src/Take.cpp >> @@ -126,7 +126,7 @@ int Take::Callback(const char* path, const struct stat >> *sb, int typeflag, struct >> return 0; >> } >> >> -void Take::ChangeOwner(string path, uid_t owner, Take::FD fd, bool >> ChangeGroup) >> +void Take::ChangeOwner(string path, uid_t owner, Take::FD& fd, bool >> ChangeGroup) >> { >> if (!ChangeGroup) >> { >> @@ -215,7 +215,7 @@ bool Take::CheckHL(string path) >> return false; >> } >> >> -bool Take::Overtake(string path, Take::FD fd) >> +bool Take::Overtake(string path, Take::FD& fd) >> { >> char * p = realpath(path.c_str(), NULL); >> > > > _______________________________________________ > Labs-l mailing list > [email protected] > https://lists.wikimedia.org/mailman/listinfo/labs-l _______________________________________________ Labs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/labs-l
