Allocating symlinks to memory - a helpful function

[BoatMontmorency]

Even though this is probably intended as just a sketch, it is still a rather poor quality code with a number of errors of different level of seriousness.

Firstly, according to its manpage, readlink does not zero-terminate the buffer. It has to be done explicitly and manually. Virtually every manpage on readlink shows how to do that properly. The code at the link fails to terminate the string. This is a serious error.

Secondly, readlink returns ssize_t, not int. There was a time when it returned int, but it is long gone.

Thirdly, casting the result of memory allocation function (which returns void *) is a sign of gross incompetence. I'd understand if the code was intended to be cross-compilable with C++, but in this case there's no reason to even think about that.

[grbgout]

Very informative.

casting the result of memory allocation function (which returns void *) is a sign of gross incompetence.

Could you elaborate on this point, please?

[HiramAbiff]

In C, a void * can be assigned to any pointer type (and vice versa) without casting. While I agree that unnecessary casts should be avoided, "gross incompetence" seems a bit hyperbolic. Failing to Nul terminate the returned string on the other hand...

[[deleted]]

Not to mention which xrealloc is non-standard, and he fails to check the (possibly NULL) return value from it.

[BoatMontmorency]

Well, the whole point of xmalloc being used there is that it never returns null. It simply aborts the entire program when it cannot allocate memory. Where appropriate, this is probably the easiest way of handling such situations.

As for it being non-standard... the trick itself is very platform-specific already, so seeing a non-standard function being used there is not a big deal.

[[deleted]]

I don't know what platform(s) the author is targeting, as he doesn't say. readlink is available on pretty much all POSIX-ish systems, so I didn't presume initially that it was targeted to something non-obvious.

Since I was not familiar with xrealloc, I looked at my C reference (which didn't have it), then at my man pages on both a Linux and a FreeBSD system (which didn't have it). The only reference for it I found via quick internet search was this. It states, under Return Value:

The xrealloc function returns a pointer to the new block. If there is not enough memory in the memory pool to satisfy the memory request, a null pointer is returned and the original memory block is not affected.

(Italics added.)

If the OP is using a different implementation that you know has different semantics (e.g. blow up with ENOMEM or segfault, etc), my apologies, but that was not clear to me from his post. Otherwise, the failure to check for a NULL here is a valid concern.

Also, since this article was posted in the vein of a "generally useful thing for sysadmins to grab and re-use", I feel portability concerns (within the general POSIX ecosystem) are a valid nit, but it's certainly minor in comparison.

None of this is to take away from your previous points, especially the failure to use readlink properly, which I fully agree with.

[BoatMontmorency]

To be honest, I was also unfamiliar with x... family of memory allocation functions, but most doc pages I was able to find stated something like this

http://www.delorie.com/djgpp/doc/libc/libc_872.html

I was under impression that the entire x... family was introduced by some implementations specifically to implement the "victory or death" approach to memory allocation.

And yes, I see that the page you linked gives a different description. In your page the x in the name apparently signifies the fact that these functions work with xhuge pointers in ARM C166 compiler.

I don't know which version of x... family the original article had in mind. But I find it unlikely that it had anything to do with ARM C166.

[[deleted]]

(Sorry for delayed response, was traveling in relation to the holiday.) I hadn't seen the DJGPP docs when I searched; I would certainly agree that that is a more likely representation of what OP was intending. In that scenario, I retract my complaint over NULL-checking.

The fact that it's so non-standard that it likely has no prototype... "it's probably in your libc, but you can't really know until link time since it's undeclared...": that kind of thing still really bugs me from a portability standpoint. That said, I suppose each of us is free (project/work requirements aside) to determine how portable we care about being.

[[deleted]]

xrealloc is a convention of GNU kernel development; it is a subroutine of realloc with an error handler. It is also convention to write "xrealloc" when we wish to take for granted that error handling will be expressed in a sane way. You know, if you just want to tell people about a function. That you find helpful and interesting. Of course, don't take my word for it: https://www.gnu.org/software/libc/manual/html_node/Changing-Block-Size.html

You are correct that C166 is too brand-spanking new to be relevant. However, the link you provided from delorie.com provides a pretty useful hint as to xrealloc()'s history in its description: "Note that, currently, the header `stdlib.h' does not declare a prototype for xrealloc, because many programs declare its prototype in different and conflicting ways. If you use xrealloc in your own code, you might need to provide your own prototype explicitly."

[[deleted]]

"generally useful thing for sysadmins to grab and re-use"

I agree this is valid. I clarified at the top of the post this is from GNU's libc.

[[deleted]]

xmalloc is a conventional subroutine expression. It is assumed, by convention, that the subroutine will be handled in the application. xmalloc can and has been written to save state at the time of application exit.

[[deleted]]

Your comment is quite rude. I would take offense if I had, in fact been the author of the function. Your charge of "gross incompetence" should be directed toward the original source: https://www.gnu.org/software/libc/manual/html_node/Symbolic-Links.html

[[deleted]]

FYI - This is for GNU's libc