improve code documentation and remove dead code

.github/workflows/test.yml

@@ -84,3 +84,35 @@ jobs:
             -Dwith-tests=true
       - name: Run distribution tests
         run: meson dist -C build
+
+  build-doxygen-docs:
+    name: Build Doxygen Docs
+    runs-on: macos-latest
+    timeout-minutes: 12
+    env:
+      HOMEBREW_NO_INSTALL_CLEANUP: 1
+      HOMEBREW_NO_AUTO_UPDATE: 1
+    steps:
+      - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
+      - name: Install dependencies
+        run: |
+          brew update
+          brew upgrade
+          brew install \
+            bstring \
+            doxygen \
+            iniparser \
+            meson
+      - name: Configure
+        run: |
+          meson setup build \
+            -Dbuildtype=debug \
+            -Dwith-docs=developer \
+            -Dwith-doxygen-strict=true \
+            -Dwith-homebrew=true
+      - name: Upload Meson log on failure
+        if: failure()
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+        with:
+          name: meson-log
+          path: build/meson-logs/meson-log.txt

CONTRIBUTING.md

@@ -175,7 +175,6 @@ You can also use the `./contrib/scripts/codefmt.sh` convenience script to the sa
   - Variable and function names should be in snake_case
   - Lists of symbols, files etc. should be sorted in alphabetic order, unless a constraint dictates otherwise
   - One variable declaration and definition per line
-  - Code documentation comments should be in kernel-doc format
   - Alphanumeric symbols in optarg should be in alphabetical order, with unary options first and parameter options second
 
 Ex.
@@ -219,6 +218,37 @@ Ex.
  */
 ```
 
+- Code documentation comments
+  - Use *Qt style* comments for documenting functions, parameters, return values, structs, enums, and macros
+  - Use `/*! ... */` to indicate multi-line comments
+  - Use `/*!< ... */` to indicate single-line comments
+  - Use a *@* prefix for [commanded tags](https://www.doxygen.nl/manual/commands.html) such as `@param` and `@returns`
+  - A multi-line function documentation comment should have a `@brief` description first,
+    followed by a blank line, followed by a more elaborate description if needed,
+    followed by `@param` and `@returns` value documentation
+  - Indicate input/output status for `@param` with \[in\],[\out\], and \[[in,out]\] tags
+  - Use Markdown for rich formatting, for instance for numbered and bulleted lists
+
+Ex.
+
+```C
+/*!
+ * @brief Copy a string from a UCS2 src to a unix char * destination, allocating a buffer
+ *
+ * @param ch destination character set
+ * @param src source UCS2 string
+ * @param dest always set at least to NULL
+ * @param destlen maximum length of destination buffer
+ *
+ * @returns The number of bytes occupied by the string in the destination
+ */
+size_t ucs2_to_charset(charset_t ch, const ucs2_t *src, char *dest,
+                       size_t destlen)
+{
+    ...
+}
+```
+
 - Padding and alignment
   - Pointer and reference operators (*, &, or ^) should be aligned to the variable name
   - One space padding after comma

bin/nad/nad.h

@@ -70,7 +70,7 @@ extern cnid_t cnid_for_paths_parent(const afpvol_t *vol, const char *path,
                                     cnid_t *did);
 extern char *utompath(const struct vol *, const char *);
 extern int convert_dots_encoding(const afpvol_t *svol, const afpvol_t *dvol,
-                                 char *path, size_t buflen);
+                                 char *path);
 
 typedef struct {
     char *p_end;/* pointer to NULL at end of path */

bin/nad/nad_cp.c

@@ -502,7 +502,7 @@ static int copy(const char *path,
 
     /* Convert basename to appropriate volume encoding */
     if (dvolume.vol->v_path
-            && (convert_dots_encoding(&svolume, &dvolume, to.p_path, MAXPATHLEN)) == -1) {
+            && (convert_dots_encoding(&svolume, &dvolume, to.p_path)) == -1) {
         SLOG("Error converting name for %s", to.p_path);
         badcp = rval = 1;
         return -1;

bin/nad/nad_util.c

@@ -92,8 +92,9 @@ void _log(enum logtype lt, char *fmt, ...)
 }
 
 /*!
- * Load volinfo and initialize struct vol
+ * @brief Load volinfo and initialize struct vol
  *
+ * @param obj    (r)  AFPObj of the current connection
  * @param path   (r)  path to evaluate
  * @param vol    (rw) structure to initialize
  *
@@ -191,21 +192,20 @@ char *utompath(const struct vol *vol, const char *upath)
 
 
 /*!
- * Convert dot encoding of basename _in place_
+ * @brief Convert dot encoding of basename _in place_
  *
  * path arg can be "[/][dir/ | ...]filename". It will be converted in place
  * possible encoding ".file" as ":2efile" which means the result will be
  * longer then the original which means provide a big enough buffer.
  *
  * @param svol   (r)  source volume
- * @param dvol   (r)  destinatio volume
+ * @param dvol   (r)  destination volume
  * @param path   (rw) path to convert _in place_
- * @param buflen (r)  size of path buffer (max strlen == buflen - 1)
  *
- * @returns 0 on sucess, -1 on error
+ * @returns 0 on success, -1 on error
  */
 int convert_dots_encoding(const afpvol_t *svol, const afpvol_t *dvol,
-                          char *path, size_t buflen _U_)
+                          char *path)
 {
     static charset_t from = (charset_t) -1;
     static char buf[MAXPATHLEN + 2];
@@ -243,7 +243,7 @@ int convert_dots_encoding(const afpvol_t *svol, const afpvol_t *dvol,
 }
 
 /*!
- * Resolves CNID of a given paths parent directory
+ * @brief Resolves CNID of a given paths parent directory
  *
  * path might be:
  * (a) relative:

doc/Doxyfile.in

@@ -949,7 +949,7 @@ WARN_LAYOUT_FILE       = YES
 # Possible values are: NO, YES, FAIL_ON_WARNINGS and FAIL_ON_WARNINGS_PRINT.
 # The default value is: NO.
 
-WARN_AS_ERROR          = NO
+WARN_AS_ERROR          = @WARN_AS_ERROR@
 
 # The WARN_FORMAT tag determines the format of the warning messages that Doxygen
 # can produce. The string should contain the $file, $line, and $text tags, which

doc/developer/dircache.md

@@ -44,9 +44,9 @@ graph TB
     FS --> ARC
     ARC --> DISK
 
-    style PC fill:#ffcccc
-    style DC fill:#ccffcc
-    style ARC fill:#ffcccc
+    style PC fill:\#ffcccc
+    style DC fill:\#ccffcc
+    style ARC fill:\#ffcccc
 ```
 
 Each cache layer uses an **LRU (Least Recently Used)** algorithm to decide what to keep in memory
@@ -201,9 +201,9 @@ flowchart TD
     Add_Cache --> Return[Return to client]
     Use_Cached --> Return
 
-    style DC_Check fill:#e6f3ff
-    style CNID_Query fill:#ffe6e6
-    style FS_Check fill:#ffffe6
+    style DC_Check fill:\#e6f3ff
+    style CNID_Query fill:\#ffe6e6
+    style FS_Check fill:\#ffffe6
 ```
 
 ### The Precedence Hierarchy
@@ -362,10 +362,10 @@ graph LR
     M3 --> R3[Reduced read IOPS]
     M4 --> R4[Higher hit ratio]
 
-    style R1 fill:#e6f3ff
-    style R2 fill:#e6f3ff
-    style R3 fill:#e6f3ff
-    style R4 fill:#e6f3ff
+    style R1 fill:\#e6f3ff
+    style R2 fill:\#e6f3ff
+    style R3 fill:\#e6f3ff
+    style R4 fill:\#e6f3ff
 ```
 
 ### Sample Statistics

doc/meson.build

@@ -37,6 +37,12 @@ if build_dev_docs and doxygen.found()
     doxyfile_conf.set('ABS_TOP_SRCDIR', meson.project_source_root())
     doxyfile_conf.set('OUTPUT_DIR', meson.current_build_dir())
 
+    if get_option('with-doxygen-strict')
+        doxyfile_conf.set('WARN_AS_ERROR', 'YES')
+    else
+        doxyfile_conf.set('WARN_AS_ERROR', 'NO')
+    endif
+
     doxyfile = configure_file(input: 'Doxyfile.in', output: 'Doxyfile', configuration: doxyfile_conf)
 
     run_command(doxygen, doxyfile, check: true)

etc/afpd/acls.c

@@ -82,10 +82,10 @@
 #ifdef HAVE_NFSV4_ACLS
 
 /*!
- * Compile access rights for a user to one file-system object
+ * @brief Compile access rights for a user to one file-system object
  *
- * This combines combines all access rights for a user to one fs-object and
- * returns the result as a Darwin allowed rights ACE.
+ * This combines all access rights for a user to one fs-object
+ * and returns the result as a Darwin allowed rights ACE.
  * This must honor trivial ACEs which are a mode_t mapping.
  *
  * @param obj            (r) handle
@@ -462,7 +462,7 @@ static uint32_t posix_permset_to_darwin_rights(acl_entry_t e, int is_dir)
 }
 
 /*!
- * Compile access rights for a user to one file-system object
+ * @brief Compile access rights for a user to one file-system object
  *
  * This combines combines all access rights for a user to one fs-object and
  * returns the result as a Darwin allowed rights ACE.
@@ -580,6 +580,8 @@ static int posix_acl_rights(const AFPObj *obj,
 }
 
 /*!
+ * @brief Convert Posix ACL permissions into access rights
+ *
  * Helper function for posix_acls_to_uaperms() to convert Posix ACL permissions
  * into access rights needed to fill ua_permissions of a FPUnixPrivs structure.
  *
@@ -624,7 +626,8 @@ static uint8_t acl_permset_to_uarights(acl_entry_t entry)
 }
 
 /*!
- * Update FPUnixPrivs for a file-system object on a volume supporting ACLs
+ * @brief Update FPUnixPrivs for a file-system object
+ * on a volume supporting ACLs
  *
  * Checks permissions granted by ACLS for a user to one fs-object and
  * updates user and group permissions in given struct maccess. As OS X
@@ -741,34 +744,8 @@ static int posix_acls_to_uaperms(const AFPObj *obj, const char *path,
     EC_EXIT;
 }
 
-#if 0
-/*!
- * Add entries of one acl to another acl
- *
- * @param aclp   (rw) destination acl where new aces will be added
- * @param acl    (r)  source acl where aces will be copied from
- *
- * @returns 0 on success, -1 on error
- */
-static int acl_add_acl(acl_t *aclp, const acl_t acl)
-{
-    EC_INIT;
-    int id;
-    acl_entry_t se, de;
-
-    for (id = ACL_FIRST_ENTRY; acl_get_entry(acl, id, &se) == 1;
-            id = ACL_NEXT_ENTRY) {
-        EC_ZERO_LOG_ERR(acl_create_entry(aclp, &de), AFPERR_MISC);
-        EC_ZERO_LOG_ERR(acl_copy_entry(de, se), AFPERR_MISC);
-    }
-
-EC_CLEANUP:
-    EC_EXIT;
-}
-#endif
-
 /*!
- * Map Darwin ACE rights to POSIX 1e perm
+ * @brief Map Darwin ACE rights to POSIX 1e perm
  *
  * We can only map few rights:
  *   DARWIN_ACE_READ_DATA                    -> ACL_READ
@@ -803,7 +780,8 @@ static acl_perm_t map_darwin_right_to_posix_permset(uint32_t darwin_ace_rights,
 }
 
 /*!
- * Add a ACL_USER or ACL_GROUP permission to an ACL, extending existing ACEs
+ * @brief  Add a ACL_USER or ACL_GROUP permission to an ACL,
+ * extending existing ACEs
  *
  * Add a permission of "type" for user or group "id" to an ACL. Scan the ACL
  * for existing permissions for this type/id, if there is one add the perm,
@@ -870,7 +848,7 @@ static int posix_acl_add_perm(acl_t *aclp, acl_tag_t type, uid_t id,
 }
 
 /*!
- * Map Darwin ACL to POSIX ACL.
+ * @brief Map Darwin ACL to POSIX ACL.
  *
  * aclp must point to a acl_init'ed acl_t or an acl_t that can e.g. contain default ACEs.
  * Mapping pecularities:
@@ -1546,10 +1524,12 @@ static int set_acl(const struct vol *vol,
 #endif /* HAVE_POSIX_ACLS */
 
 /*!
- * Checks if a given UUID has requested_rights(type darwin_ace_rights) for path.
+ * @brief Checks if a given UUID has requested_rights (type darwin_ace_rights)
+ * for path.
  *
  * Note: this gets called frequently and is a good place for optimizations !
  *
+ * @param obj              (r) AFP object
  * @param vol              (r) volume
  * @param dir              (rw) directory
  * @param path             (r) path to filesystem object
@@ -1972,7 +1952,7 @@ int afp_setacl(AFPObj *obj, char *ibuf, size_t ibuflen _U_, char *rbuf _U_,
  ********************************************************************/
 
 /*!
- * map ACL to user maccess
+ * @brief map ACL to user maccess
  *
  * This is the magic function that makes ACLs usable by calculating
  * the access granted by ACEs to the logged in user.

etc/afpd/afp_config.c

@@ -52,7 +52,7 @@
 #include "dircache.h"
 
 /*!
- * Safe string to integer conversion with bounds checking
+ * @brief Safe string to integer conversion with bounds checking
  *
  * @param str         String to convert
  * @param param_name  Parameter name for error messages
@@ -106,7 +106,7 @@ static int safe_atoi(const char *str, const char *param_name,
 }
 
 /*!
- * Free and cleanup config and DSI
+ * @brief Free and cleanup config and DSI
  *
  * "dsi" can be NULL in which case all DSI objects and the config object is freed,
  * otherwise its an afpd session child and only any unneeded DSI objects are freed
@@ -166,7 +166,7 @@ void configfree(AFPObj *obj, DSI *dsi)
 
 
 /*!
- * Get everything running
+ * @brief Get everything running
  */
 int configinit(AFPObj *dsi_obj, AFPObj *asp_obj)
 {

etc/afpd/catsearch.c

@@ -531,7 +531,7 @@ static int rslt_add(const AFPObj *obj, struct vol *vol, struct path *path,
         "./../.AppleDouble/.AppleDB/Network Trash Folder/TheVolumeSettingsFolder/TheFindByContentFolder/.AppleDesktop/.Parent/"
 
 /*!
- * This function performs a filesystem search
+ * @brief This function performs a filesystem search
  *
  * Uses globals c1, c2, the search criteria
  *
@@ -780,12 +780,13 @@ static int catsearch(const AFPObj *obj,
 } /* catsearch() */
 
 /*!
- * This function performs a CNID db search
+ * @brief This function performs a CNID db search
  *
  * Uses globals c1, c2, the search criteria
+ * Always searches the whole volume, not a subtree
  *
+ * @param obj       (r)  AFP connection object
  * @param vol       (r)  volume we are searching on ...
- * @param dir       (rw) directory we are starting from ...
  * @param uname     (r)  UNIX name of object to search
  * @param rmatches  (r)  maximum number of matches we can return
  * @param pos       (r)  position we've stopped recently
@@ -796,7 +797,6 @@ static int catsearch(const AFPObj *obj,
  */
 static int catsearch_db(const AFPObj *obj,
                         struct vol *vol,
-                        struct dir *dir _U_,
                         const char *uname,
                         int rmatches,
                         uint32_t *pos,
@@ -1159,7 +1159,7 @@ static int catsearch_afp(AFPObj *obj _U_, char *ibuf, size_t ibuflen,
             && (vol->v_flags & AFPVOL_SEARCHDB))
         /* we've got a name and it's a dbd volume, so search CNID database */
     {
-        ret = catsearch_db(obj, vol, vol->v_root, uname, rmatches, &catpos[0],
+        ret = catsearch_db(obj, vol, uname, rmatches, &catpos[0],
                            rbuf + 24, &nrecs, &rsize, ext);
     } else
         /* perform a slow filesystem tree search */