Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Modern C++ pattern for ugly C struct allocation

Tags:

c

c++11

I'm making an ioctl call from C++ into a driver I don't own/maintain, and I'm trying to sort out if there's a clean, "safe-ish" mechanism to deal with some of the ugly struct allocation required.

Slimmed down version of some structures involved

// IOCTL expects an instance of this structure "first"
typedef struct {
   int param1;
   int param2;
} s_ioctl_request;

//... followed by an instance of this.  If attr_length
// is > sizeof(s_attr_header), more data is allowed to follow.
typedef struct {
   uint32_t attr_length;
   uint32_t attr_type;
} s_attr_header;

// Example that uses more data than just the header.
typedef struct {
   s_attr_header   hdr;
   uint32_t attr_param;
} s_attr_type1;

// Another example.
typedef struct {
   s_attr_header   hdr;
   uint32_t attr_param1;
   uint32_t attr_param2;
} s_attr_type2;

The ioctl requires that s_ioctl_request be immediately followed by an s_attr_header, or other struct containing it, where attr_length is set to the size of the outer struct in bytes.

In C, to write a wrapper for the ioctl it would be done via something along these lines:

int do_ugly_ioctl(int fd, int p1, int p2, s_attr_header * attr)
{  
   int res;      
   // Allocate enough memory for both structures.
   s_ioctl_request *req = malloc( sizeof(*req) + attr->hdr.attr_length );

   // Copy the 2nd, (variable length) structure after the first.
   memcpy( ((char*)req) + sizeof(*req), attr, attr->hdr.attr_length);

   // Modify params as necessary
   req->param1 = p1;
   req->param2 = p2;

   // Make the driver call, free mem, and return result.
   res = ioctl(fd, SOME_IOCTL_ID, req);
   free(req);
   return res;
}

// Example invocation.
s_attr_type1 a1;
a1.hdr.attr_length = sizeof(a1);
a1.hdr.attr_type   = 1;
do_ugly_ioctl(fd, 10, 20, &a1);

A couple options I'm thinking of, are:

  1. Throw modern C++-isms out the window, and do exactly what I've shown above.

  2. Allocate the storage with a std::vector, then do ugly casts with the resulting std::vector::data() pointer so at least I'm not doing new[] / delete[] or malloc / free.

  3. Create a unique wrapper method for each s_attr_type* that uses its own "special" struct. This seems "safest", i.e. least likely for the user of the wrapper method to screw it up. And bonus points, allows pass-by-ref.

Method #3 example:

int do_ugly_ioctl(fd, int param1, int param2, s_attr_type2& attr){
  struct RequestData {
      s_ioctl_request ioreq;
      s_attr_type2    attr;
  };
  RequestData r;
  r.ioreq.param1 = param1;
  r.ioreq.param2 = param2;
  r.attr         = attr;
  r.attr.hdr.attr_length = sizeof(attr); // Might as well enforce this here.
  ioctl(fd, SOME_IOCTL_ID, (void*) &r);
}

So I guess some questions here are:

  • Is it "worth it" to C++-ize a solution to this problem? (as opposed to relying on the more error-prone C impl).

  • If I go with method #3 or similar, is there anything that I can do with <type_traits> to make a template of this function and only accept structs with an s_attr_header as the first member?

  • Any other brilliant ideas?

like image 776
Brian McFarland Avatar asked Jun 27 '16 22:06

Brian McFarland


1 Answers

Totally worth it, and your solution is quite nice. You might want to declare your structures as packed (there are compiler extensions to achieve this) to avoid having extra padding when combining multiple structures.

You can also set the size of the structure within the constructor.

struct RequestData
{
      RequestData() : ioreq{}, attr{}
      {
        attr.hdr.attr_length =  sizeof(attr);
      }
      s_ioctl_request ioreq;
      s_attr_type2    attr;
 };

concerning your second question, you could split the assignment in two, it's not too nice but it's easy and if you pass something without a correct header, it will lead to a compiler error:

template<typename Attr>
int do_ugly_ioctl(fd, int param1, int param2, Attr& attr){
  struct RequestData {
      s_ioctl_request ioreq;
      Attr    attr;
  };
  RequestData r;
  r.ioreq.param1 = param1;
  r.ioreq.param2 = param2;
  s_attr_header hdr = Attr.hdr; //this will lead to compilation error if the type is not what we expect
  (void) hdr;
  r.attr     = attr;
  r.attr.hdr.attr_length = sizeof(attr); // Might as well enforce this here.
  ioctl(fd, SOME_IOCTL_ID, (void*) &r);
}
like image 169
dau_sama Avatar answered Nov 13 '22 06:11

dau_sama