vfs smb, scan: read every smb:// URL from the configured server

The VFS reads the configured server whatever host a URL names, so a
scan or refresh no longer compares the host: a playlist saved under an
older server address refreshes after the SMB server setting changes,
and the check takes no name lookup. A share other than the configured
one is still refused.

vfs_test checks URLs of other hosts, unresolvable ones included, pass
on their share alone.

docs/retroarch.6

@@ -84,7 +84,7 @@ Does --scan-refresh for every playlist that has saved scan settings, as Refresh
 .IP
 These run in the order given, and one that fails does not stop the rest.
 Each prints a totals line on stdout, or its error on stderr.
-An smb:// folder must be on the server and share set in the SMB settings, and one that cannot be read says why.
+An smb:// folder is read from the server set in the SMB settings and must be on the share set there, and one that cannot be read says why.
 Without --menu, RetroArch exits when they are done and does not load content given on the command line.
 The exit status is then 1 if any failed, 0 if any added an entry, and 2 if none added anything; a skipped playlist does not count.
 With --menu, they run while the menu does.

intl/msg_hash_us.h

@@ -32581,10 +32581,6 @@ MSG_HASH(
    MSG_SCAN_SMB_NO_SERVER,
    "SMB server address not configured"
    )
-MSG_HASH(
-   MSG_SCAN_SMB_HOST_DIFFERS,
-   "host differs from the SMB server setting"
-   )
 MSG_HASH(
    MSG_SCAN_SMB_SHARE_DIFFERS,
    "share differs from the SMB share setting"

libretro-common/vfs/vfs_implementation_smb.c

@@ -681,131 +681,29 @@ enum smb_failure smb_url_failure(const char *url)
    return SMB_FAILURE_UNKNOWN;
 }
 
-/* The host in @s, a URL past smb:// or the server setting: up to the
- * first '/', without a user@ prefix, a :port suffix or IPv6 brackets. */
-static bool smb_host_split(const char *s, char *host, size_t len)
-{
-   const char *end = strchr(s, '/');
-   const char *p;
-   size_t      n;
-   if (!end)
-      end = s + strlen(s);
-   for (p = s; p < end; p++)
-      if (*p == '@')
-         s = p + 1;
-   if (*s == '[')
-   {
-      s++;
-      p = s;
-      while (p < end && *p != ']')
-         p++;
-      if (p == end)
-         return false;
-      end = p;
-   }
-   /* one colon starts a port, more are a bare IPv6 address */
-   else if (   (p = (const char*)memchr(s, ':', (size_t)(end - s)))
-            && !memchr(p + 1, ':', (size_t)(end - p - 1)))
-      end = p;
-   n = (size_t)(end - s);
-   if (!n || n >= len)
-      return false;
-   memcpy(host, s, n);
-   host[n] = '\0';
-   return true;
-}
-
-static bool smb_addr_equal(const struct addrinfo *a, const struct addrinfo *b)
-{
-   if (a->ai_family != b->ai_family || !a->ai_addr || !b->ai_addr)
-      return false;
-   switch (a->ai_family)
-   {
-      case AF_INET:
-         return !memcmp(&((const struct sockaddr_in*)a->ai_addr)->sin_addr,
-               &((const struct sockaddr_in*)b->ai_addr)->sin_addr,
-               sizeof(struct in_addr));
-#ifdef HAVE_INET6
-      case AF_INET6:
-         return !memcmp(&((const struct sockaddr_in6*)a->ai_addr)->sin6_addr,
-               &((const struct sockaddr_in6*)b->ai_addr)->sin6_addr,
-               sizeof(struct in6_addr));
-#endif
-      default:
-         break;
-   }
-   return false;
-}
-
-/* Whether @a and @b share an address; a failed lookup shares none. */
-static bool smb_hosts_meet(const char *a, const char *b)
-{
-   struct addrinfo  hints;
-   struct addrinfo *ra = NULL;
-   struct addrinfo *rb = NULL;
-   struct addrinfo *x;
-   struct addrinfo *y;
-   bool             met = false;
-   if (!network_init())
-      return false;
-   memset(&hints, 0, sizeof(hints));
-   hints.ai_family   = AF_UNSPEC;
-   hints.ai_socktype = SOCK_STREAM;
-   if (getaddrinfo_retro(a, NULL, &hints, &ra) == 0 && ra)
-   {
-      if (getaddrinfo_retro(b, NULL, &hints, &rb) == 0 && rb)
-      {
-         for (x = ra; x && !met; x = x->ai_next)
-            for (y = rb; y && !met; y = y->ai_next)
-               met = smb_addr_equal(x, y);
-         freeaddrinfo_retro(rb);
-      }
-      freeaddrinfo_retro(ra);
-   }
-   return met;
-}
-
-/* The setting goes through the same split as the URL, so the URL the
- * menu builds from it (smb://<server>/<share>/...) always matches. */
+/* The host a URL names is not compared: every URL is read from the
+ * configured server, so a playlist saved under an older server
+ * address still refreshes. */
 enum smb_url_result smb_url_check(const char *url, char *detail, size_t len)
 {
    vfs_cfg_hdr_t *h;
    const struct smb_cfg_snap *snap = smb_cfg_enter(&h);
-   char server[256];
-   char share[256];
-   char want[256];
+   enum smb_url_result r = SMB_URL_OK;
    char got[256];
    detail[0] = '\0';
-   server[0] = '\0';
-   share[0]  = '\0';
-   /* copies, so the lookup below holds no settings copy */
-   if (snap)
-   {
-      strlcpy(server, snap->server_address, sizeof(server));
-      strlcpy(share, snap->share, sizeof(share));
-   }
-   smb_cfg_leave(h);
-   if (!*server)
-      return SMB_URL_NO_SERVER;
-   if (   !string_starts_with(url, SMB_PREFIX)
-       || !smb_host_split(url + STRLEN_CONST(SMB_PREFIX), got, sizeof(got))
-       || !smb_host_split(server, want, sizeof(want))
-       || (   !string_is_equal_case_insensitive(got, want)
-           && !smb_hosts_meet(got, want)))
-   {
-      strlcpy(detail, server, len);
-      return SMB_URL_HOST_DIFFERS;
-   }
+   if (!snap || !*snap->server_address)
+      r = SMB_URL_NO_SERVER;
    /* an empty URL share differs too: the VFS would read the
     * configured share in its place */
-   if (   *share
-       && (   !smb_url_share(url, got, sizeof(got))
-           || !string_is_equal_case_insensitive(got, share)))
+   else if (   *snap->share
+            && (   !smb_url_share(url, got, sizeof(got))
+                || !string_is_equal_case_insensitive(got, snap->share)))
    {
-      strlcpy(detail, share, len);
-      return SMB_URL_SHARE_DIFFERS;
+      strlcpy(detail, snap->share, len);
+      r = SMB_URL_SHARE_DIFFERS;
    }
-   return SMB_URL_OK;
+   smb_cfg_leave(h);
+   return r;
 }
 
 /* A private connection for @key, owned by the caller; the outcome is

libretro-common/vfs/vfs_implementation_smb.h

@@ -78,19 +78,18 @@ enum smb_failure
  * leave either's code. */
 enum smb_failure smb_url_failure(const char *url);
 
-/* Whether a URL names the server and share of the settings. */
+/* Whether a URL can be read under the settings. */
 enum smb_url_result
 {
    SMB_URL_OK = 0,
    SMB_URL_NO_SERVER,
-   SMB_URL_HOST_DIFFERS,
    SMB_URL_SHARE_DIFFERS     /* a share is configured and the URL's differs */
 };
 
-/* Checks @url's host and share against the settings; on a difference
- * @detail gets the configured server or share, else "". Hosts compare
- * as text, ignoring case, then by address: this may block on a name
- * lookup, so never call it from the main thread. */
+/* Checks @url's share against the settings, ignoring case; on a
+ * difference @detail gets the configured share, else "". The URL's
+ * host is not compared: the configured server is always read. No
+ * I/O. */
 enum smb_url_result smb_url_check(const char *url, char *detail, size_t len);
 
 /* File operations */

msg_hash.h

@@ -29980,7 +29980,6 @@ enum msg_hash_enums
    MSG_PLAYLIST_MANAGER_REFRESH_ALL_DONE,
 
    MSG_SCAN_SMB_NO_SERVER,
-   MSG_SCAN_SMB_HOST_DIFFERS,
    MSG_SCAN_SMB_SHARE_DIFFERS,
    MSG_SCAN_SMB_CANNOT_RESOLVE,
    MSG_SCAN_SMB_CANNOT_CONNECT,

samples/network/smb_retro/vfs_test.c

@@ -67,7 +67,7 @@ static int expect_failure(struct smb_settings *cfg, const char *share,
 
 static const char *check_name(enum smb_url_result r)
 {
-   static const char *const names[] = { "ok", "no server", "host differs", "share differs" };
+   static const char *const names[] = { "ok", "no server", "share differs" };
    return (unsigned)r < sizeof(names) / sizeof(*names) ? names[r] : "?";
 }
 
@@ -113,7 +113,8 @@ static int check_menu(struct smb_settings *cfg, const char *server,
 }
 
 /* SMB_CHECK_URLS: URLs against the settings, before network_init(), with
- * no server; localhost must resolve to 127.0.0.1. */
+ * no server. The host is never compared, so any host passes and only
+ * the share decides. */
 static int check_urls(void)
 {
    struct smb_settings cfg;
@@ -125,16 +126,15 @@ static int check_urls(void)
    memcpy(long_host, "smb://", 6);
    memset(long_host + 6, 'a', 300);
    strcpy(long_host + 306, "/share/roms");
-   /* the same text resolves nowhere: no lookup when only case differs */
    bad += check_url(&cfg, "nas.INVALID", "share", "smb://NAS.invalid/share/roms", SMB_URL_OK, "");
    bad += check_url(&cfg, "127.0.0.1", "share", "smb://localhost/share/roms", SMB_URL_OK, "");
    bad += check_url(&cfg, "localhost", "", "smb://127.0.0.1/share", SMB_URL_OK, "");
-   bad += check_url(&cfg, "127.0.0.1", "share", "smb://127.0.0.2/share/roms", SMB_URL_HOST_DIFFERS, "127.0.0.1");
-   bad += check_url(&cfg, "127.0.0.1", "share", "smb://nosuch.invalid/share/roms", SMB_URL_HOST_DIFFERS, "127.0.0.1");
-   bad += check_url(&cfg, "nosuch.invalid", "share", "smb://127.0.0.1/share/roms", SMB_URL_HOST_DIFFERS, "nosuch.invalid");
-   bad += check_url(&cfg, "127.0.0.1", "share", "smb:///share/roms", SMB_URL_HOST_DIFFERS, "127.0.0.1");
-   bad += check_url(&cfg, "127.0.0.1", "share", "smb://[::1/share/roms", SMB_URL_HOST_DIFFERS, "127.0.0.1");
-   bad += check_url(&cfg, "127.0.0.1", "share", long_host, SMB_URL_HOST_DIFFERS, "127.0.0.1");
+   bad += check_url(&cfg, "127.0.0.1", "share", "smb://127.0.0.2/share/roms", SMB_URL_OK, "");
+   bad += check_url(&cfg, "127.0.0.1", "share", "smb://nosuch.invalid/share/roms", SMB_URL_OK, "");
+   bad += check_url(&cfg, "nosuch.invalid", "share", "smb://127.0.0.1/share/roms", SMB_URL_OK, "");
+   bad += check_url(&cfg, "127.0.0.1", "share", "smb:///share/roms", SMB_URL_OK, "");
+   bad += check_url(&cfg, "127.0.0.1", "share", "smb://[::1/share/roms", SMB_URL_OK, "");
+   bad += check_url(&cfg, "127.0.0.1", "share", long_host, SMB_URL_OK, "");
    bad += check_url(&cfg, "127.0.0.1", "share", "smb://[email protected]:445/share/roms", SMB_URL_OK, "");
    bad += check_url(&cfg, "127.0.0.1:445", "share", "smb://127.0.0.1/share/roms", SMB_URL_OK, "");
    bad += check_url(&cfg, "::1", "share", "smb://[::1]/share/roms", SMB_URL_OK, "");
@@ -148,7 +148,7 @@ static int check_urls(void)
    bad += check_url(&cfg, "127.0.0.1", "share", "smb://127.0.0.1", SMB_URL_SHARE_DIFFERS, "share");
    bad += check_url(&cfg, "127.0.0.1", "share", "smb://127.0.0.1/", SMB_URL_SHARE_DIFFERS, "share");
    bad += check_url(&cfg, "127.0.0.1", "share", "smb://127.0.0.1//roms", SMB_URL_SHARE_DIFFERS, "share");
-   bad += check_url(&cfg, "127.0.0.1", "share", "smb://127.0.0.2/other", SMB_URL_HOST_DIFFERS, "127.0.0.1");
+   bad += check_url(&cfg, "127.0.0.1", "share", "smb://127.0.0.2/other", SMB_URL_SHARE_DIFFERS, "share");
    bad += check_url(&cfg, "", "share", "smb://127.0.0.1/share/roms", SMB_URL_NO_SERVER, "");
    bad += check_menu(&cfg, "127.0.0.1", "share", "roms");
    bad += check_menu(&cfg, "127.0.0.1", "share", "/roms/nes");

samples/tasks/database/database_scan_test.c

@@ -9732,8 +9732,6 @@ static void smb_reason_lane(const char *root, bool threaded)
    } cases[] = {
       { "no server",      SMB_URL_NO_SERVER,     SMB_FAILURE_UNKNOWN,
             "",          MSG_SCAN_SMB_NO_SERVER },
-      { "host differs",   SMB_URL_HOST_DIFFERS,  SMB_FAILURE_UNKNOWN,
-            "127.0.0.1", MSG_SCAN_SMB_HOST_DIFFERS },
       { "share differs",  SMB_URL_SHARE_DIFFERS, SMB_FAILURE_UNKNOWN,
             "retro",     MSG_SCAN_SMB_SHARE_DIFFERS },
       { "cannot resolve", SMB_URL_OK,            SMB_FAILURE_RESOLVE,
@@ -9906,8 +9904,6 @@ static void smb_refresh_lane(const char *root, uint32_t crc_a,
    } cases[] = {
       { "no server",      SMB_URL_NO_SERVER,     SMB_FAILURE_UNKNOWN,
             "",          MSG_SCAN_SMB_NO_SERVER },
-      { "host differs",   SMB_URL_HOST_DIFFERS,  SMB_FAILURE_UNKNOWN,
-            "127.0.0.1", MSG_SCAN_SMB_HOST_DIFFERS },
       { "share differs",  SMB_URL_SHARE_DIFFERS, SMB_FAILURE_UNKNOWN,
             "retro",     MSG_SCAN_SMB_SHARE_DIFFERS },
       { "cannot resolve", SMB_URL_OK,            SMB_FAILURE_RESOLVE,
@@ -10020,15 +10016,15 @@ static void smb_refresh_lane(const char *root, uint32_t crc_a,
    }
 
    /* --scan-refresh=NAME skips it the same way. */
-   smb_stub_check   = SMB_URL_HOST_DIFFERS;
+   smb_stub_check   = SMB_URL_SHARE_DIFFERS;
    smb_stub_failure = SMB_FAILURE_UNKNOWN;
-   smb_stub_detail  = "127.0.0.1";
+   smb_stub_detail  = "retro";
    if (!(items = (task_dbscan_item_t*)calloc(1, sizeof(*items))))
       goto fixture;
    refresh_item(&items[0], TASK_DBSCAN_ITEM_REFRESH, "s_smb");
    ran = refresh_run(items, 1, out, err, true);
-   sprintf(line, "s_smb: skipped: %s 127.0.0.1 (%s)",
-         msg_hash_to_str_us(MSG_SCAN_SMB_HOST_DIFFERS), url);
+   sprintf(line, "s_smb: skipped: %s retro (%s)",
+         msg_hash_to_str_us(MSG_SCAN_SMB_SHARE_DIFFERS), url);
    sprintf(what, "smb refresh %s: by name, skipped", q);
    check(   ran && chain_seen == 1
          && chain_saw(0, "s_smb", TASK_DBSCAN_SKIPPED, -1)

tasks/task_database.c

@@ -4568,10 +4568,9 @@ static bool manual_scan_walk_within_budget(void *ud)
 }
 
 #ifdef HAVE_SMBCLIENT
-/* True when the smb:// URL @dir names no server, or another host or
- * share than the SMB settings: @reason says which, and @detail the
- * setting a mismatch names, else "".  Stats nothing, but may look up
- * a name: not for the main thread. */
+/* True when no SMB server is set, or the smb:// URL @dir names
+ * another share than the SMB settings: @reason says which, and
+ * @detail the configured share on a mismatch, else "".  No I/O. */
 static bool scan_smb_url_refused(const char *dir,
       enum msg_hash_enums *reason, char *detail, size_t len)
 {
@@ -4581,9 +4580,6 @@ static bool scan_smb_url_refused(const char *dir,
       case SMB_URL_NO_SERVER:
          *reason = MSG_SCAN_SMB_NO_SERVER;
          return true;
-      case SMB_URL_HOST_DIFFERS:
-         *reason = MSG_SCAN_SMB_HOST_DIFFERS;
-         return true;
       case SMB_URL_SHARE_DIFFERS:
          *reason = MSG_SCAN_SMB_SHARE_DIFFERS;
          return true;
@@ -6194,7 +6190,7 @@ static void dbscan_path_handler(retro_task_t *task)
       else if (!*s)
          free(s);
 #ifdef HAVE_SMBCLIENT
-      /* Never stat'd: no server, or another host or share */
+      /* Never stat'd: no server, or another share */
       else if (   string_starts_with_size(s, "smb://", STRLEN_CONST("smb://"))
                && scan_smb_url_refused(s, &prep->smb_reason,
                      prep->smb_detail, sizeof(prep->smb_detail)))
@@ -6775,7 +6771,7 @@ static void dbscan_load_handler(retro_task_t *task)
       if (!playlist_scan_refresh_enabled(prep->playlist))
          prep->load = DBSCAN_LOAD_NO_SETTINGS;
 #ifdef HAVE_SMBCLIENT
-      /* Never stat'd: no server, or another host or share */
+      /* Never stat'd: no server, or another share */
       else if (   string_starts_with_size(dir, "smb://",
                      STRLEN_CONST("smb://"))
                && scan_smb_url_refused(dir, &prep->smb_reason,