Changeset 55581 in vbox

Timestamp:

May 1, 2015 11:01:59 AM (9 years ago)

Author:

vboxsync

Message:

VBoxServiceControlSession.cpp: A few more constructive code review comments.

File:

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceControlSession.cpp

-              r55580
+              r55581
         uint32_t uFlagsRemRec = 0;
         bool fRecursive = false;
+/** @todo r=bird: Unnecessary variable fRecursive.  You can check for
+ * DIRREMOVE_FLAG_RECURSIVE directly in the flags when deciding which API to
+ * call. */
         if (!(uFlags & ~DIRREMOVE_FLAG_VALID_MASK))
 …
                 fRecursive = true;
+            }
+/** @todo r=bird: Understand how APIs you use work (read docs, check constant,
+ * check code). If you check the actual values of RTDIRRMREC_F_CONTENT_AND_DIR
+ * and RTDIRRMREC_F_CONTENT_ONLY, you'd notice that the first one is 0 and the
+ * second is 1.  This code is a little confused about how it all works, though
+ * it ends up doing the right thing as if by accident almost. */
             if (uFlags & DIRREMOVE_FLAG_CONTENT_AND_DIR)
+            {
 …
                            szDir, uFlags, fRecursive);
+/** @todo r=bird: Convoluted code flow. It would be shorter and easier to
+ * read if you moved this code up and into the flags-are-valid if body. */
         if (RT_SUCCESS(rc))
+        {

Note: See TracChangeset for help on using the changeset viewer.