Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

readlinknul(): Minor tweaks #1107

Merged
merged 2 commits into from
Dec 10, 2024
Merged

Conversation

alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Nov 3, 2024


Revisions:

v1b
  • Rebase
$ git range-diff alx/master..gh/readlink shadow/master..readlink 
1:  56975bbd = 1:  1df0343c lib/fs/readlink/readlinknul.h: readlinknul(): Simplify the return value
2:  dacf42bd = 2:  13c7d4c9 lib/fs/readlink/readlinknul.h: readlinknul(): Silence warning
v1c
  • Rebase
$ git range-diff gh/master..gh/readlink shadow/master..readlink 
1:  1df0343c = 1:  9c99500d lib/fs/readlink/readlinknul.h: readlinknul(): Simplify the return value
2:  13c7d4c9 = 2:  340b5544 lib/fs/readlink/readlinknul.h: readlinknul(): Silence warning
v1d
  • Rebase
$ git range-diff master..gh/readlink shadow/master..readlink 
1:  9c99500d = 1:  26597716 lib/fs/readlink/readlinknul.h: readlinknul(): Simplify the return value
2:  340b5544 = 2:  96367947 lib/fs/readlink/readlinknul.h: readlinknul(): Silence warning
v2
  • Be more like readlink(2). [@hallyn ]
$ git range-diff shadow/master gh/readlink readlink 
1:  26597716 < -:  -------- lib/fs/readlink/readlinknul.h: readlinknul(): Simplify the return value
2:  96367947 = 1:  ec20d793 lib/fs/readlink/readlinknul.h: readlinknul(): Silence warning
-:  -------- > 2:  44f5100e lib/fs/readlink/: readlinknul(): Fix return type
v2b
  • Rebase
$ git range-diff master..gh/readlink shadow/master..readlink 
1:  ec20d793 = 1:  eebe88a6 lib/fs/readlink/readlinknul.h: readlinknul(): Silence warning
2:  44f5100e = 2:  b29b9242 lib/fs/readlink/: readlinknul(): Fix return type
v2c
  • Rebase
$ git range-diff master..gh/readlink shadow/master..readlink 
1:  eebe88a6 = 1:  f3ff9482 lib/fs/readlink/readlinknul.h: readlinknul(): Silence warning
2:  b29b9242 = 2:  321d4d44 lib/fs/readlink/: readlinknul(): Fix return type
v2d
  • Rebase
$ git range-diff master..gh/readlink shadow/master..readlink 
1:  f3ff9482 = 1:  a3656c70 lib/fs/readlink/readlinknul.h: readlinknul(): Silence warning
2:  321d4d44 = 2:  effb3787 lib/fs/readlink/: readlinknul(): Fix return type
v2e
  • Rebase
$ git range-diff master..gh/readlink shadow/master..readlink 
1:  a3656c70 = 1:  fbc78bca lib/fs/readlink/readlinknul.h: readlinknul(): Silence warning
2:  effb3787 = 2:  1c3d86c8 lib/fs/readlink/: readlinknul(): Fix return type
v3
  • Rename local variables, and return the signed one. [@hallyn ]
$ git range-diff --creation-factor=99 shadow/master gh/readlink readlink 
1:  fbc78bca ! 1:  302f144a lib/fs/readlink/readlinknul.h: readlinknul(): Silence warning
    @@ lib/fs/readlink/readlinknul.h: inline int readlinknul(const char *restrict link,
      readlinknul(const char *restrict link, char *restrict buf, size_t size)
      {
     -  ssize_t  len;
    -+  size_t   len;
    -+  ssize_t  r;
    ++  size_t   ulen;
    ++  ssize_t  slen;
      
     -  len = readlink(link, buf, size);
     -  if (len == -1)
    -+  r = readlink(link, buf, size);
    -+  if (r == -1)
    ++  slen = readlink(link, buf, size);
    ++  if (slen == -1)
                return -1;
      
    -+  len = r;
    -   if (len == size) {
    +-  if (len == size) {
    ++  ulen = slen;
    ++  if (ulen == size) {
                stpcpy(&buf[size-1], "");
                errno = E2BIG;
    +           return -1;
    +   }
    + 
    +-  stpcpy(&buf[len], "");
    +-  return len;
    ++  stpcpy(&buf[ulen], "");
    ++
    ++  return slen;
    + }
    + 
    + 
2:  1c3d86c8 ! 2:  42fc0318 lib/fs/readlink/: readlinknul(): Fix return type
    @@ lib/fs/readlink/readlinknul.h
     +inline ssize_t
      readlinknul(const char *restrict link, char *restrict buf, size_t size)
      {
    -   size_t   len;
    +   size_t   ulen;

hallyn
hallyn previously requested changes Dec 2, 2024
lib/fs/readlink/readlinknul.h Outdated Show resolved Hide resolved
@alejandro-colomar alejandro-colomar force-pushed the readlink branch 2 times, most recently from 9636794 to 44f5100 Compare December 2, 2024 10:07
@alejandro-colomar alejandro-colomar force-pushed the readlink branch 3 times, most recently from 321d4d4 to effb378 Compare December 6, 2024 12:09
@alejandro-colomar
Copy link
Collaborator Author

Queued after the release of 4.17.0.

@alejandro-colomar alejandro-colomar marked this pull request as draft December 6, 2024 12:17
@alejandro-colomar
Copy link
Collaborator Author

Actually, this fixes a minor bug. I'll re-open. @hallyn , please merge when you're happy.

@alejandro-colomar alejandro-colomar marked this pull request as ready for review December 6, 2024 16:15
Copy link
Collaborator

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@alejandro-colomar
Copy link
Collaborator Author

Thanks! Feel free to merge.

@alejandro-colomar alejandro-colomar dismissed hallyn’s stale review December 9, 2024 16:27

Addressed. The function now matches the comment.

Use a temporary variable to silence a sign-mismatch diagnostic.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
Fixes: 419ce14 (2024-11-01, "lib/fs/readlink/: readlinknul(): Add function")
Cc: Serge Halyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar alejandro-colomar force-pushed the readlink branch 2 times, most recently from 1c3d86c to 42fc031 Compare December 10, 2024 03:24
@hallyn hallyn merged commit 8821d3f into shadow-maint:master Dec 10, 2024
8 checks passed
@alejandro-colomar alejandro-colomar deleted the readlink branch December 10, 2024 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Simpler A good issue for a new beginner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants