Use of a Union to avoid Dynamic Allocation Headaches

Question

I'm curious if it's a good idea to use a union when accessing Win32 APIs that return variable length structures, to avoid manually managing the allocated memory.

Consider the following:

void displayServices(std::wostream& log, std::tr1::shared_ptr<void> manager, LPENUM_SERVICE_STATUS currentServiceToDisplay, DWORD number, bool whitelist)
{
    static union //Don't care about being thread safe
    {
     QUERY_SERVICE_CONFIG svcConfig;
     unsigned char bufferSizer[8000];
    };
    for(DWORD idx = 0; idx < number; currentServiceToDisplay++, idx++)
    {
     DWORD garbage = 0;
     std::tr1::shared_ptr<void> currentServiceHandle(
      OpenService(reinterpret_cast<SC_HANDLE>(manager.get()), currentServiceToDisplay->lpServiceName, SERVICE_QUERY_CONFIG),
      serviceCloser);
     QueryServiceConfig(reinterpret_cast<SC_HANDLE>(currentServiceHandle.get()),
          &svcConfig, 8000, &garbage);
     //Use svcConfig for something here.
    }
}

Are there any major issues with this?

Answer 1

Other than using a lot more stack space than necessary, no.

Update:

Yeah, totally missed the static keyword, sorry.

It will work fine as long as you don't need to use it from multiple threads, and there's no chance of the function being re-entrant.

Answer 2

There aren't any major issues that I know of. But rather than using a union, I think just declaring the buffer and then using pointers to get the structure you want for it will be more clear.

I guess what I don't really get from your example is what the point of doing it this way is.

Edit: What I mean is, do this:

void displayServices(std::wostream& log, std::tr1::shared_ptr<void> manager, LPENUM_SERVICE_STATUS currentServiceToDisplay, DWORD number, bool whitelist)
{
    unsigned char bufferSizer[8000];
    QUERY_SERVICE_CONFIG *pSvcConfig = reinterpret_cast<QUERY_SERVICE_CONFIG*>(bufferSizer);

    for(DWORD idx = 0; idx < number; currentServiceToDisplay++, idx++)
    {
     DWORD garbage = 0;
     std::tr1::shared_ptr<void> currentServiceHandle(
      OpenService(reinterpret_cast<SC_HANDLE>(manager.get()), currentServiceToDisplay->lpServiceName, SERVICE_QUERY_CONFIG),
      serviceCloser);
     QueryServiceConfig(reinterpret_cast<SC_HANDLE>(currentServiceHandle.get()),
          pSvcConfig, sizeof(bufferSizer), &garbage);
     //Use pSvcConfig for something here.
    }
}

To me, at least, this is more clear.

Answer 3

Another way, just using a static array of your base type:

void displayServices(std::wostream& log, std::tr1::shared_ptr<void> manager, LPENUM_SERVICE_STATUS currentServiceToDisplay, DWORD number, bool whitelist)
{
    static QUERY_SERVICE_CONFIG svcConfig[8000/sizeof(QUERY_SERVICE_CONFIG)]; // Or just pick an arbitrary number

    for(DWORD idx = 0; idx < number; currentServiceToDisplay++, idx++)
    {
        DWORD garbage = 0;
        std::tr1::shared_ptr<void> currentServiceHandle(
                OpenService(reinterpret_cast<SC_HANDLE>(manager.get()), currentServiceToDisplay->lpServiceName, SERVICE_QUERY_CONFIG),
                serviceCloser);
        QueryServiceConfig(reinterpret_cast<SC_HANDLE>(currentServiceHandle.get()),
                                                svcConfig, sizeof(svcConfig), &garbage);
        //Use svcConfig for something here.
    }
}

Answer 4

One problem with your union approach is that the whole idea of using an union is rather forced and completely unnecessary. All you seem to want to do is the replace dynamic memory allocation with a local static buffer of some "large" size. Then just do it explicitly

unsigned char buffer[8000];    
QUERY_SERVICE_CONFIG *svcConfig = (QUERY_SERVICE_CONFIG *) buffer;
...
QueryServiceConfig(reinterpret_cast<SC_HANDLE>(currentServiceHandle.get()), 
  svcConfig, sizeof buffer, &garbage);
// Use `svcConfig` here, keeping in mind it is a pointer now

What you were trying to achive by using union specifically is not clear to me. Do you think that the approach with the union is somehow more elegant? I don't think so.

Update: It has been noted in one of the comments that this approach allegedly might not work with a compiler performing strict aliasing optimizations. The comment is absolutely incorrect for more than one reason. Firstly, the above approach does not rely on any aliasing-dependent behavior at all, since all access is supposed to be performed through svcConfig pointer and svcConfig pointer only. Secondly, strict aliasing optimizations are never performed if the aliased data is a character array. The language specification explicitly permits aliased access through character arrays, which is why all optimizing compilers disable their strict aliasing optimizations when a character array is involved. Any compiler that fails to do so is broken and thus not worth consideration.