Remove dead beef #1230

alejandro-colomar · 2025-02-24T22:25:02Z

We didn't even have prototypes for these APIs since long ago, when the prototypes were removed, but misteriously the implementations remained.

Both glibc and musl provide getspnam(3), so this file was effectively being ignored by the compiler. Just remove it.

Also remove the check for getspnam, which isn't used elsewhere.

Fixes: 0ee095a (2007-10-07; "[svn-upgrade] Integrating new upstream version, shadow (4.0.7)")
Closes: #1228

Revisions:

v2

Remove other dead code found by @zeha

$ git range-diff master gh/sh sh
1:  22f49d87 = 1:  22f49d87 configure.ac, lib/shadow.c, po/: Remove dead file
-:  -------- > 2:  036f5d7e configure.ac, contrib/, src/: Remove obsolete check
-:  -------- > 3:  97856757 configure.ac: Remove unused check for futimes(3)

v2b

Remove dead code I forgot to remove in v2.

$ git range-diff master gh/sh sh
1:  22f49d87 = 1:  22f49d87 configure.ac, lib/shadow.c, po/: Remove dead file
2:  036f5d7e ! 2:  4bb6df21 configure.ac, contrib/, src/: Remove obsolete check
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    configure.ac, contrib/, src/: Remove obsolete check
    +    configure.ac, contrib/, src/: Remove dead code
     
         Both glibc and musl provide getusershell(3).  It's an API from 4.3BSD,
         according to the manual page, so let's assume it exists everywhere that
    @@ Commit message
         Reported-by: Chris Hofstaedtler <zeha@debian.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    +    f
    +
      ## configure.ac ##
     @@ configure.ac: dnl shadow now uses the libc's shadow implementation
      AC_CHECK_HEADER([shadow.h],,[AC_MSG_ERROR([You need a libc with shadow.h])])
    @@ src/chsh.c: static bool shell_is_listed (const char *sh)
        }
        endusershell ();
     -#else
    -+
    -   char *buf = NULL;
    -   FILE *fp;
    -   size_t n = 0;
    +-  char *buf = NULL;
    +-  FILE *fp;
    +-  size_t n = 0;
    + 
    +-  fp = fopen (SHELLS_FILE, "r");
    +-  if (NULL == fp) {
    +-          return false;
    +-  }
    +-
    +-  while (getline (&buf, &n, fp) != -1) {
    +-          if (buf[0] != '/') {
    +-                  continue;
    +-          }
    +-
    +-          if (streq(buf, sh)) {
    +-                  found = true;
    +-                  break;
    +-          }
    +-  }
    +-
    +-  free(buf);
    +-  fclose (fp);
    +-#endif
    +   return found;
    + }
    + #endif /* with HAVE_VENDORDIR */
3:  97856757 = 3:  9c7bd91a configure.ac: Remove unused check for futimes(3)

v2c

Remove stray 'f' in commit message (it was a glitch of rebasing). [@zeha]

$ git range-diff master gh/sh sh
1:  22f49d87 = 1:  22f49d87 configure.ac, lib/shadow.c, po/: Remove dead file
2:  4bb6df21 ! 2:  11addd70 configure.ac, contrib/, src/: Remove dead code
    @@ Commit message
         Reported-by: Chris Hofstaedtler <zeha@debian.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    -    f
    -
      ## configure.ac ##
     @@ configure.ac: dnl shadow now uses the libc's shadow implementation
      AC_CHECK_HEADER([shadow.h],,[AC_MSG_ERROR([You need a libc with shadow.h])])
3:  9c7bd91a = 3:  239bfe65 configure.ac: Remove unused check for futimes(3)

v2d

Rebase

$ git range-diff master..gh/sh shadow/master..sh
1:  22f49d87 = 1:  7e0a3f7b configure.ac, lib/shadow.c, po/: Remove dead file
2:  11addd70 = 2:  c15827cd configure.ac, contrib/, src/: Remove dead code
3:  239bfe65 = 3:  c140a255 configure.ac: Remove unused check for futimes(3)

v2e

Improve commit subject line

$ git range-diff shadow/master gh/sh sh
1:  7e0a3f7b ! 1:  d1c7dbb2 configure.ac, lib/shadow.c, po/: Remove dead file
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    configure.ac, lib/shadow.c, po/: Remove dead file
    +    lib/shadow.c, configure.ac, po/: Remove dead file <lib/shadow.c>
     
         We didn't even have prototypes for these APIs since long ago, when the
         prototypes were removed, but misteriously the implementations remained.
2:  c15827cd = 2:  70478719 configure.ac, contrib/, src/: Remove dead code
3:  c140a255 = 3:  8e57d32a configure.ac: Remove unused check for futimes(3)

v3f

Fix commit subject line

$ git range-diff shadow/master gh/sh sh
1:  d1c7dbb2 ! 1:  c7e47809 lib/shadow.c, configure.ac, po/: Remove dead file <lib/shadow.c>
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/shadow.c, configure.ac, po/: Remove dead file <lib/shadow.c>
    +    lib/, configure.ac, po/: Remove dead file <lib/shadow.c>
     
         We didn't even have prototypes for these APIs since long ago, when the
         prototypes were removed, but misteriously the implementations remained.
2:  70478719 = 2:  36766600 configure.ac, contrib/, src/: Remove dead code
3:  8e57d32a = 3:  8c91195e configure.ac: Remove unused check for futimes(3)

v3g

Rebase

$ git rd
1:  c7e47809 = 1:  da4897d0 lib/, configure.ac, po/: Remove dead file <lib/shadow.c>
2:  36766600 = 2:  bffc2d96 configure.ac, contrib/, src/: Remove dead code
3:  8c91195e = 3:  86f4b814 configure.ac: Remove unused check for futimes(3)

v3h

Rebase

$ git rd
1:  da4897d0 = 1:  1f81b8a1 lib/, configure.ac, po/: Remove dead file <lib/shadow.c>
2:  bffc2d96 = 2:  a05a605c configure.ac, contrib/, src/: Remove dead code
3:  86f4b814 = 3:  a38eb49f configure.ac: Remove unused check for futimes(3)

v4

Remove file. [@ikerexxe ]

$ git rd
1:  1f81b8a1 = 1:  1f81b8a1 lib/, configure.ac, po/: Remove dead file <lib/shadow.c>
2:  a05a605c = 2:  a05a605c configure.ac, contrib/, src/: Remove dead code
3:  a38eb49f = 3:  a38eb49f configure.ac: Remove unused check for futimes(3)
-:  -------- > 4:  4daece33 lib/xgetspnam.c: Remove dead file

v5

Revert to v3h.

$ git rd
1:  1f81b8a1 = 1:  1f81b8a1 lib/, configure.ac, po/: Remove dead file <lib/shadow.c>
2:  a05a605c = 2:  a05a605c configure.ac, contrib/, src/: Remove dead code
3:  a38eb49f = 3:  a38eb49f configure.ac: Remove unused check for futimes(3)
4:  4daece33 < -:  -------- lib/xgetspnam.c: Remove dead file

v5b

Rebase

$ git rd
1:  1f81b8a1 = 1:  ea69a2cd lib/, configure.ac, po/: Remove dead file <lib/shadow.c>
2:  a05a605c = 2:  be6e67f3 configure.ac, contrib/, src/: Remove dead code
3:  a38eb49f = 3:  220b9a0b configure.ac: Remove unused check for futimes(3)

zeha · 2025-02-25T10:21:11Z

4bb6df2 commit message has a stray "f" at the end

alejandro-colomar · 2025-02-25T10:32:07Z

4bb6df2 commit message has a stray "f" at the end

Thanks! That was a glitch of squashing a commit with a dummy message, and forgetting to remove the name.

alejandro-colomar · 2025-05-26T12:22:53Z

@ikerexxe This one is really trivial. I'll push it higher in the priority list. Please review.

ikerexxe

While reviewing the getspnam related code, I found that lib/xgetspnam.c still contains a reference to it. I guess we should remove that file too.

alejandro-colomar · 2025-05-26T15:05:25Z

While reviewing the getspnam related code, I found that lib/xgetspnam.c still contains a reference to it. I guess we should remove that file too.

Hmmm, thanks! I'll push a commit removing that file. Let's see if it passes CI cleanly.

alejandro-colomar · 2025-05-26T15:09:47Z

While reviewing the getspnam related code, I found that lib/xgetspnam.c still contains a reference to it. I guess we should remove that file too.

Hmmm, thanks! I'll push a commit removing that file. Let's see if it passes CI cleanly.

Ahh, that file defines xgetspnam, not getspnam (see XFUNCTION_NAME in xgetXXbyYY.c. So I'll revert that.

We didn't even have prototypes for these APIs since long ago, when the prototypes were removed, but misteriously the implementations remained. Both glibc and musl provide getspnam(3), so this file was effectively being ignored by the compiler. Just remove it. Also remove the check for getspnam, which isn't used elsewhere. Fixes: 0ee095a (2007-10-07; "[svn-upgrade] Integrating new upstream version, shadow (4.0.7)") Closes: <shadow-maint#1228> Signed-off-by: Alejandro Colomar <alx@kernel.org>

Both glibc and musl provide getusershell(3). It's an API from 4.3BSD, according to the manual page, so let's assume it exists everywhere that we would care, even if it's not in POSIX. Reported-by: Chris Hofstaedtler <zeha@debian.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>

Reported-by: Chris Hofstaedtler <zeha@debian.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>

hallyn

lgtm, but let's wait until Iker gives a final ok before merging.

ikerexxe · 2025-05-27T07:42:19Z

Ahh, that file defines xgetspnam, not getspnam (see XFUNCTION_NAME in xgetXXbyYY.c. So I'll revert that.

I see. Sorry for the noise.

ikerexxe

LGTM! Let's merge the changes.

alejandro-colomar · 2025-05-27T08:34:31Z

Thank you both!

Ahh, that file defines xgetspnam, not getspnam (see XFUNCTION_NAME in xgetXXbyYY.c. So I'll revert that.

I see. Sorry for the noise.

No problem. That code is rather convoluted and difficult to read correctly. That's why I do what I do, indeed. :-)

alejandro-colomar requested review from hallyn and ikerexxe February 24, 2025 22:25

alejandro-colomar force-pushed the sh branch 2 times, most recently from d959af8 to 22f49d8 Compare February 24, 2025 22:47

alejandro-colomar changed the title ~~configure.ac, lib/shadow.c: Remove dead file~~ Remove dead beef Feb 24, 2025

alejandro-colomar force-pushed the sh branch from 9785675 to 9c7bd91 Compare February 24, 2025 23:28

alejandro-colomar force-pushed the sh branch from 9c7bd91 to 239bfe6 Compare February 25, 2025 10:30

alejandro-colomar force-pushed the sh branch 3 times, most recently from 8e57d32 to 8c91195 Compare March 3, 2025 21:01

alejandro-colomar force-pushed the sh branch from 8c91195 to 86f4b81 Compare April 10, 2025 10:52

alejandro-colomar force-pushed the sh branch from 86f4b81 to a38eb49 Compare May 3, 2025 12:14

alejandro-colomar mentioned this pull request May 4, 2025

4.18 release plan #1257

Closed

ikerexxe reviewed May 26, 2025

View reviewed changes

alejandro-colomar force-pushed the sh branch from 4daece3 to a38eb49 Compare May 26, 2025 15:10

alejandro-colomar added 3 commits May 26, 2025 23:40

configure.ac: Remove unused check for futimes(3)

220b9a0

Reported-by: Chris Hofstaedtler <zeha@debian.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>

alejandro-colomar force-pushed the sh branch from a38eb49 to 220b9a0 Compare May 26, 2025 21:40

hallyn approved these changes May 27, 2025

View reviewed changes

ikerexxe approved these changes May 27, 2025

View reviewed changes

ikerexxe merged commit 1b93681 into shadow-maint:master May 27, 2025
10 checks passed

alejandro-colomar deleted the sh branch May 27, 2025 08:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove dead beef #1230

Remove dead beef #1230

Uh oh!

alejandro-colomar commented Feb 24, 2025 •

edited

Loading

Uh oh!

zeha commented Feb 25, 2025

Uh oh!

alejandro-colomar commented Feb 25, 2025

Uh oh!

alejandro-colomar commented May 26, 2025

Uh oh!

ikerexxe left a comment

Uh oh!

alejandro-colomar commented May 26, 2025

Uh oh!

alejandro-colomar commented May 26, 2025

Uh oh!

hallyn left a comment

Uh oh!

ikerexxe commented May 27, 2025

Uh oh!

ikerexxe left a comment

Uh oh!

Uh oh!

alejandro-colomar commented May 27, 2025

Uh oh!

Uh oh!

Remove dead beef #1230

Remove dead beef #1230

Uh oh!

Conversation

alejandro-colomar commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeha commented Feb 25, 2025

Uh oh!

alejandro-colomar commented Feb 25, 2025

Uh oh!

alejandro-colomar commented May 26, 2025

Uh oh!

ikerexxe left a comment

Choose a reason for hiding this comment

Uh oh!

alejandro-colomar commented May 26, 2025

Uh oh!

alejandro-colomar commented May 26, 2025

Uh oh!

hallyn left a comment

Choose a reason for hiding this comment

Uh oh!

ikerexxe commented May 27, 2025

Uh oh!

ikerexxe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alejandro-colomar commented May 27, 2025

Uh oh!

Uh oh!

alejandro-colomar commented Feb 24, 2025 •

edited

Loading