[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#12411: [PATCH] A better Directory Lister example



tags 12411 + patch
thanks

Hi, attached is a patch that gives a better Directory Lister example.
Ian's concerns are addressed as follows:

> * it does not check the return values from closedir or puts;

Now it does.

> * it does not fflush stdout (so that errors writing stdout will not be
>   detected unless they happen more than one stdio buffer length from
>   the end of the output data);

I have declined to address this, since this example is mainly concerned
with using the libc directory reading functions, not with handling stdout
errors.

> * it prints the error message about failing to open the directory to
>   stdout instead of stderr;

The current version of the info file uses perror(), which, as far as I
know, print to stderr, not stdout.

> * when the opendir fails it doesn't print the value of errno.

I believe using perror() gives a sufficient representation of the value of
errno.

> Some people might say `hey, it's only an example, it doesn't matter if
> it's crap'.  However, programmers using this example when writing
> their own code will probably replicate many of these mistakes,
> producing programs with unreliable and unhelpful error behaviours.

I agree, except that an example starts to lose its effectiveness to convey
its intended message if real-life issues start to intrude upon it beyond
what is necessary. Using fflush on stdout is one point that IMHO is beyond
the scope of this example, although I do agree on the other points (to
emphasize that one should always check the return code of syscalls and
library calls, etc).

> I suppose we can forgive it for ignoring its arguments, rather than
> checking that it has none.

I'm tempted to also address this in my proposed fix to the example, but as
I said, this is getting near the point where it starts detracting from the
point of the example. I personally regard it as extremely bad practice to
declare main() as main(void); nevertheless, the stated purpose of this
example is that it lists files in the current directory, and as such, it
seems superfluous to declare and check for argc and argv. The official
glibc maintainers can, of course, add this additional check to the example
should they so choose.


T

-- 
LINUX = Lousy Interface for Nefarious Unix Xenophobes.
--- libc.info-24.ORIG	2002-12-26 21:47:28.000000000 -0500
+++ libc.info-24	2002-12-26 21:54:23.000000000 -0500
@@ -498,8 +498,17 @@
        if (dp != NULL)
          {
            while (ep = readdir (dp))
-             puts (ep->d_name);
-           (void) closedir (dp);
+             {
+               if (puts (ep->d_name) == EOF)
+                 {
+                   perror ("Unable to write to stdout");
+                   return 0;
+                 }
+             }
+           if (closedir (dp) == -1)
+             {
+               perror ("Error while reading from directory");
+             }
          }
        else
          perror ("Couldn't open the directory");

Reply to: