On 12/ 2/16 01:25 PM, Ali Bahrami wrote:
I'm currently searching code to see what the impacts are, but I suspect that there are none, and that we should follow FreeBSD's lead. They've done us a service by already testing out the viability. The reason for using unsigned long was undoubtedly so that pre-K&R C compilers can compile the code. gelf was invented at a time in history where ansi C was still pretty new, and not everyone was using it. 'unsigned long' was the pre-ansi idiom for "generic pointer" that could be used in either mode. All of the code I've found in Solaris using these functions so far, and there is very little, treat the return value as a pointer, and even compare it to NULL, so switching to 'void *' would be transparent and binary compatible. I'll follow up to this as soon as I'm done digging. In the meantime, might I suggest that you make this change in a test environment, do a 'make world', and see if you hit any issues?
Hi Mark, I'm done with my experiments, and I think that we should adopt 'void *' as the return type for these functions. That might take awhile, but I see no reason why the folks doing the Windows port shouldn't move ahead immediately, using a few ifdefs, which can be later removed when we all catch up. I already described the machine-level reasons why I think this change is safe. 'void *' and 'unsigned long' generate the same machine code for systems where sizeof(long) == sizeof(pointer) which generally holds for unix, and on any systems that are sill likely to be running. For these systems, these are equivalent alternative syntaxes that generate the same machine code, and and here should be no binary level incompatibilities. The FreeBSD experiment provides real world evidence of this. That's the theory, here are my experiments... The first thing to report is that use of these 2 functions is very rare. In Solaris, they are called once each in a small set of programs: - The mcs/strip/elfcompress programs, which are really one program, hardlinked together, using the argv[0] trick to figure out what they are at runtime. - A demo program we ship under /usr/demo/ELF - A couple of we use to build Solaris, but which are not shipped. In all of these cases, our code calls them as such: if (gelf_newehdr(elf, class) == NULL) bail(); and then accesses them using gelf get/set routines. We know that the return value is a valid pointer, but we never access it via that pointer, probably because that requires the programmer to care about which ELFCLASS they're dealing with. To do that defeats the only real reason to use GElf rather than just calling the "real" class-specific libelf routines. For that reason, it would have made sense for these functions to return a boolean error/success indication rather than a pointer. I don't think we should make that change at this late date --- it's just an observation in passing. I also googled for these APIs, and found very little of note, other than manpages. I made the ['void *' change, to the <gelf.h> header, and to libelf, and then without modifying any calling code, did a full build of our core OS workspace. There were no errors. I then installed those bits on my build machine, and used it to do a second full build, and that one was also completely clean. I therefore have no concerns about this regarding any code that we produce. I also grepped the sources for a majority of the open source that we deliver (including gcc and binutils), and found that none of it calls these functions. If they did, I'm confident that they would "just work", with the possible exception of some easily fixed C++. However, as nothing calls it, there nothing to be concerned about. In your environment, you may have some different consumers, so I'll wait to hear what you find, but 'void *' seems OK from here. In the final analysis, our main concern is to keep the various libelf's unified, so we'll go along with the group consensus. - Ali _______________________________________________ elfutils-devel mailing list -- elfutils-devel@lists.fedorahosted.org To unsubscribe send an email to elfutils-devel-le...@lists.fedorahosted.org