> On Apr 5, 2017, at 2:48 AM, Nicholas Marriott wrote:
>
>> On Tue, Apr 04, 2017 at 05:03:26PM -0500, Scott Cheloha wrote:
>>
>> [...]
>>
>> In the current code, however, you could have insufficient permissions
>> for a part of the path (EPERM), or an I/O failure (EIO), but otherwise
>> specify a valid symbolic link, and still get the described behavior,
>> which seems wrong to me.
>>
>> If the documented "say nothing and exit 1" behavior when the target
>> is not a symbolic link is sacred, then maybe this snippet is better:
>
> There are likely to be existing scripts that rely on this behaviour.
The EPERM/EIO silent failure? Okay.
> [...]
>
> I think it would be better not to change the behaviour, except possibly
> for ENAMETOOLONG.
Agreed. Adjusted proposal:
Eliminate the strlen(3) call because both realpath(3) and readlink(2) do
length checking anyway. Leverage err(3)/warn(3) and the error string for
ENAMETOOLONG to report when this happens in lieu of the custom error
message used now.
Aside from the format of the error message when the file name argument
exceeds PATH_MAX bytes, the only change in behavior is that readlink(1)
*without* the -f flag now complains about the file name being too long
if a portion of the path name exceeds NAME_MAX bytes. To see this,
compare
readlink $(perl -e 'print "z"x256')
with/without the patch.
While here, make some other style(9)-type changes.
Index: readlink.c
===================================================================
RCS file: /cvs/src/usr.bin/readlink/readlink.c,v
retrieving revision 1.27
diff -u -p -r1.27 readlink.c
--- readlink.c 9 Oct 2015 01:37:08 -0000 1.27
+++ readlink.c 6 Apr 2017 00:13:34 -0000
@@ -32,17 +32,17 @@
#include <limits.h>
#include <stdio.h>
#include <stdlib.h>
-#include <string.h>
#include <unistd.h>
-static void usage(void);
+static __dead void usage(void);
int
main(int argc, char *argv[])
{
char buf[PATH_MAX];
- int n, ch, nflag = 0, fflag = 0;
- extern int optind;
+ int ch, fflag, n, nflag;
+
+ fflag = nflag = 0;
if (pledge("stdio rpath", NULL) == -1)
err(1, "pledge");
@@ -64,30 +64,25 @@ main(int argc, char *argv[])
if (argc != 1)
usage();
- n = strlen(argv[0]);
- if (n > PATH_MAX - 1) {
- fprintf(stderr,
- "readlink: filename longer than PATH_MAX-1 (%d)\n",
- PATH_MAX - 1);
- exit(1);
- }
-
if (fflag) {
if (realpath(argv[0], buf) == NULL)
err(1, "%s", argv[0]);
} else {
- if ((n = readlink(argv[0], buf, sizeof buf-1)) < 0)
- exit(1);
+ if ((n = readlink(argv[0], buf, sizeof(buf) - 1)) == -1) {
+ if (errno == ENAMETOOLONG)
+ warn("%s", argv[0]);
+ return 1;
+ }
buf[n] = '\0';
}
printf("%s", buf);
if (!nflag)
putchar('\n');
- exit(0);
+ return 0;
}
-static void
+static __dead void
usage(void)
{
(void)fprintf(stderr, "usage: readlink [-fn] file\n");
