system/nxpkg: Address follow-up review findings.

Shorten the installed-database lock description and centralize pkg_sync() cleanup, as requested in review. Replace repeated protocol literals with named constants and preserve errno before cleanup calls can overwrite it.

Store the synchronized catalog source in the catalog itself so the catalog and its relative-artifact base are committed atomically. Continue reading the former sidecar format for upgrade compatibility, and normalize array-form catalogs before adding the private source field.

Compare numeric version prefixes without strtol() overflow and retain lexical comparison of suffixes.

Assisted-by: Codex:gpt-5
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>

Author: aviralgarg05

system/nxpkg/pkg_install.c

@@ -78,37 +78,8 @@ static int pkg_install_acquire_lock(FAR const char *name, FAR char *path,
  * Name: pkg_install_acquire_installed_lock
  *
  * Description:
- *   The per-package lock above (pkg_install_acquire_lock()) only ever
- *   protects one package's own version directory/manifest - it says
- *   nothing about the single shared installed-packages database
- *   (instpkg.jsn) that every install/uninstall/rollback reads, modifies
- *   in memory, and writes back as a whole.  Two of those operations for
- *   *different* packages (their own per-package locks don't conflict)
- *   can each load a snapshot of that shared file, add/change their own
- *   entry, and save - and whichever one saves last wins, silently
- *   discarding whatever the other one had just added.  This is exactly
- *   what "a previously-installed package vanishes from `nxpkg list`
- *   with no error" looks like, without needing any corrupted file or
- *   non-atomic write at all: the write path is already atomic
- *   (pkg_store_write_text_atomic() writes to a temp file, fsyncs, then
- *   renames), so the *file* is always internally consistent - it just
- *   might be a consistent snapshot that's missing an entry a concurrent
- *   operation already believed it had durably saved.
- *
- *   A separate, single global lock file (independent of any specific
- *   package's own lock) closes that window: acquire it right before
- *   pkg_metadata_load_installed() and hold it until immediately after
- *   pkg_metadata_save_installed(), so that whole read-modify-write
- *   sequence is atomic with respect to every other caller of this
- *   function, regardless of which package(s) they're touching.
- *
- *   Blocking (bounded retry loop) rather than instant-EBUSY like the
- *   per-package lock: the critical section this protects is a handful
- *   of local SD-card JSON operations with no network I/O in it, so a
- *   real holder releases it within milliseconds - a caller should wait
- *   that out rather than fail an otherwise-healthy install/uninstall/
- *   rollback just because another one's shared-db update was in
- *   flight at the same instant.
+ *   Acquire the global installed-database lock.  This serializes the
+ *   read-modify-write sequence used by install, uninstall, and rollback.
  *
  ****************************************************************************/
 

system/nxpkg/pkg_metadata.c

@@ -24,6 +24,7 @@
  * Included Files
  ****************************************************************************/
 
+#include <ctype.h>
 #include <errno.h>
 #include <stdlib.h>
 #include <string.h>
@@ -400,41 +401,63 @@ static int pkg_metadata_parse_installed_entry(
 static int pkg_metadata_version_token_cmp(FAR const char *lhs,
                                           FAR const char *rhs)
 {
-  long leftnum;
-  long rightnum;
-  FAR char *leftend;
-  FAR char *rightend;
   FAR const char *cmpleft;
   FAR const char *cmpright;
+  FAR const char *leftdigits;
+  FAR const char *rightdigits;
+  size_t leftlen;
+  size_t rightlen;
   int ret;
 
-  leftnum = strtol(lhs, &leftend, 10);
-  rightnum = strtol(rhs, &rightend, 10);
+  leftdigits = lhs;
+  rightdigits = rhs;
+  while (isdigit((unsigned char)*leftdigits))
+    {
+      leftdigits++;
+    }
+
+  while (isdigit((unsigned char)*rightdigits))
+    {
+      rightdigits++;
+    }
 
-  if (leftend != lhs && rightend != rhs)
+  if (leftdigits != lhs && rightdigits != rhs)
     {
-      if (leftnum < rightnum)
+      while (*lhs == '0' && lhs + 1 < leftdigits)
+        {
+          lhs++;
+        }
+
+      while (*rhs == '0' && rhs + 1 < rightdigits)
+        {
+          rhs++;
+        }
+
+      leftlen = (size_t)(leftdigits - lhs);
+      rightlen = (size_t)(rightdigits - rhs);
+      if (leftlen < rightlen)
         {
           return -1;
         }
 
-      if (leftnum > rightnum)
+      if (leftlen > rightlen)
         {
           return 1;
         }
 
-      /* Equal numeric prefixes don't make the tokens equal if what
-       * follows the number differs - e.g. "1a" and "1b" both parse as
-       * 1, but are documented to compare lexically by their non-numeric
-       * component.  Falling through to "equal" here (as this used to)
-       * silently treated any such pair as the same version, which is
-       * exactly backwards for tokens whose whole point is to be
-       * distinguishable.  Compare what's left after the numeric prefix
-       * - leftend/rightend - lexically instead of falling through.
-       */
+      ret = memcmp(lhs, rhs, leftlen);
+      if (ret < 0)
+        {
+          return -1;
+        }
+
+      if (ret > 0)
+        {
+          return 1;
+        }
 
-      cmpleft = leftend;
-      cmpright = rightend;
+      cmpleft = leftdigits;
+      cmpright = rightdigits;
     }
   else
     {