[Coming late to this discussion, since I'm not on -info]

As other have pointed out, there is an snprintf.c implementation in src/util. This defines afs_vsnprintf and afs_snprintf, as well as vsnprintf and snprintf for systems that do not already have these. This is a "fairly complete" implementation -- it doesn't support the i18n feature of specifying positional parameters, some of the newer ISO descriptions, or correct return count if the buffer is too small. But it does support features not found in every sprintf, such as %llu and %lld formats for 64-bit integers.

In trying to add LFS to the fileserver, I've needed to be able to display integers that might be either 32- or 64-bit, depending on compilation options. Having to choose formats based on compilation options becomes messy. The cleanest solution I've found, and what I've implemented, is to make cast these values to afs_intmax_t or afs_uintmax_t, which are guaranteed to be correctly formatted using %lld or %llu, respectively, whether or not the AFS_64BIT_ENV is set. But the best way to guarantee this is to control the implementation of afs_vsnprintf, especially since we will need something like this on some architectures anyway.

A number of changes to CVS lately have converted uses of sprintf to afs_snprintf in the fileserver and volserver. It turns out that some of the uses of sprintf are problematic: There are cases of %Ld being used -- I believe that is a GNU extension. I found one buffer overrun involving deleting volumes... Who knows what else lurks? So it seems to me that moving to snprintf and similar is necessary to ensure safety.

The question at hand is then, to attempt to use a system snprintf, or always use our own afs_snprintf? I think that most of the time, when snprintf is used, it is to generate a string that will be used next to log a message, or as a file name for a file system operation. The system snprintf might, maybe, be a little more efficient, but I'm not sure it matters in this context. Meanwhile, we have to maintain our own snprintf implementation for systems that don't have it, and we have to maintain a test program to verify that the system snprintf is robust enough, and to get correct types of afs_intmax_t. That seems like a lot of overhead for (maybe) marginal gain.

So it seems to me that we should always use our own afs_snprintf. The only question I have: Is the version we have in src/util robust enough to replace sprintf throughout the codebase? It seems to be for src/vol, src/volser, and src/viced, but I haven't gone through other parts of the code.

/Lindsay

Russ Allbery wrote:

Switching to devel (should have done that before when I started attaching
code).



-- R. Lindsay Todd email: [EMAIL PROTECTED] Senior Systems Programmer phone: 518-276-2605 Rensselaer Polytechnic Institute fax: 518-276-2809 Troy, NY 12180-3590 WWW: http://www.rpi.edu/~toddr


_______________________________________________ OpenAFS-devel mailing list [EMAIL PROTECTED] https://lists.openafs.org/mailman/listinfo/openafs-devel

Reply via email to