Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #94165

Re: Generic read/write function - casting vs union

Show all headers | View raw

Noob <root@127.0.0.1> writes:

> On 22/09/2016 17:35, Tim Rentsch wrote:
>
>> Something crucial has been left out, specifically the definitions
>> of the wrapper functions that call access().  Based on the above
>> outline it looks like read() and write() simply make (or would
>> make) a local union object, and then call access() with the
>> address of that object.  But if that is true there is no point to
>> incrementing buf->in (or buf->out) since the value is then just
>> thrown away.  (And the void* types for 'buf' in the original
>> prototypes do nothing to unmuddy the water.)  Can you please
>> supply all the relevant details?  Generally my answers are
>> better if I don't have to shoot in the dark.
>
> The crucial missing piece of information is that the contents
> of the buffer need to be "unswizzled" because what is stored
> on the device differs from what is shown to upper layers.
>
> [elaboration]
>
> I hope my description was clear (enough).

Yes I think so.  To be sure, let me play back what I understood you
to be saying (with some slight variations in some of the names).

There are some already defined, pre-existing low-level functions:

 extern void basic_read(  void *device,       uint8_t *area, int length );
 extern void basic_write( void *device, const uint8_t *area, int length );
 extern void skip_forward( void *device, int skip_amount );

You are tasked with writing two higher level functions that will
call the low-level functions:

 extern void swizzled_read(  void *device,       uint8_t *area, int length );
 extern void swizzled_write( void *device, const uint8_t *area, int length );

Because the two higher level functions are basically the same except
for whether reading or writing is done, you want to factor out the
common code so these two functions can be written as simple wrappers
calling a common routine (here the arguments READ and WRITE are
meant as constants, eg, enumeration names):

 void
 swizzled_read( void *device, uint8_t *buffer, int length ){
     swizzled_read_or_write( READ, device, buffer, length );
 }

 void
 swizzled_write( void *device, const uint8_t *buffer, int length ){
     swizzled_read_or_write( WRITE, device, buffer, length );
 }

and the question is how to arrange that.  Is that a fairly accurate
summary?  If not then I have misunderstood something.


> So my implementation looks more like this:
>
> void foo_generic(void *dev, uint8_t *buf, int len, int dir)
> {
>   elem_dispatch(dev, buf,  12, dir); buf += 12;
>   skip_forward(dev, 30);
>
>   elem_dispatch(dev, buf,  15, dir); buf += 15;
>   skip_forward(dev, 33);
>
>   elem_dispatch(dev, buf,  17, dir); buf += 17;
>   skip_forward(dev, 22);
>
>   /*** snip ***/
> }
>
> And of course elem_dispatch calls either elem_read or elem_write
> based on the dir parameter.  But this solution requires casting
> away the constness of the write buffer.
>
> So it was suggested to me to use a union-based solution.

IMO a union-based solution is an inferior choice.  Let's try going
forward on the swizzled_read_or_write() approach, for now being a
bit vague on just what types are used, and see what we get:

 static void
 swizzled_read_or_write( Direction way, Device d, Base buffer, int length ){
     read_or_write_then_skip( way, d, &buffer, 12, 30 );
     read_or_write_then_skip( way, d, &buffer, 15, 33 );
     read_or_write_then_skip( way, d, &buffer, 17, 22 );
     /* ... etc ... */
     (void) &length;  /* in what way is 'length' supposed to be used? */
 }

 static void
 read_or_write_then_skip( Direction way, Device d, Base *pb, int n, int skip ){
     Base buffer = *pb;
     assert( way == READ || way == WRITE );
     if(  way == READ  )  basic_read( d, buffer, n );
     else                basic_write( d, buffer, n );
     *pb += n;
     skip_forward( d, skip );
 }

This scheme works except in one case (for writes) a 'const' is
needed and in the other case (for reads) 'const' cannot be present.
The approach you describe casts away constness on the write() path.
Suppose we don't do that, but instead define Base this way, so
const is used in most places:

 typedef const uint8_t *Base;

Now everything works except the final call to basic_read().  There
we need to take out the constness that was added in the outermost
call:

 static void
 read_or_write_then_skip( Direction way, Device d, Base *pb, int n, int skip ){
     Base buffer = *pb;
     assert( way == READ || way == WRITE );
     if(  way == READ  )  basic_read( d, (uint8_t *) buffer, n );
     else                basic_write( d,             buffer, n );
     *pb += n;
     skip_forward( d, skip );

     /* Note: revised to cast argument to basic_read() */
 }

This alternative may be more satisfactory to you reviewers.  The
constness of the buffer for writing is preserved.  There is a cast
to take away constness of an argument to basic_read(), but that
constness was added in a higher level (but still interior) call.
To verify correctness, we need observe just a few facts:

  1.  The cast happens only when 'way == READ' is true;

  2.  The interior functions swizzled_read_or_write() and
      read_or_write_then_skip() are static, so their callers
      can be determined by examining just the one .c file;

  3.  The only function that calls swizzled_read_or_write()
      with a first argument of READ is the outer function
      swizzled_read();  and

  4.  The parameter of swizzled_read() that is used to supply
      the argument value that will eventually be casted is
      appropriately non-const.

Given what I understand of what you need to do, this approach is
what I suggest, along with the correctness argument summarized
above.  It is of course possible to find other ways to solve the
problem, including ones that don't use either unions or casts, but
they will likely be clunkier, harder to follow, or more error prone
(and possibly all three).  I don't endorse casting lightly;  in
this case I think using a cast is a better option than any other
approach I've looked at.

For completeness here is the whole source for my example.  (Please
ignore minor style issues, eg, typedef'ing a pointer type, which
some people find objectionable.)  This source should give a clean
compile as a single stand-alone file.


/****** Start here *********************************************************/

#include <stdint.h>

extern void basic_read(  void *device,       uint8_t *area, int length );
extern void basic_write( void *device, const uint8_t *area, int length );
extern void skip_forward( void *device, int skip_amount );

extern void swizzled_read(  void *device,       uint8_t *area, int length );
extern void swizzled_write( void *device, const uint8_t *area, int length );


/* Everything that follows is local to the .c file */
#include <assert.h>

typedef enum { READ, WRITE } Direction;
typedef void *Device;
typedef const uint8_t *Base;

static void swizzled_read_or_write( Direction, void *, Base, int );
static void read_or_write_then_skip( Direction, Device, Base *, int, int );


void
swizzled_read( void *device, uint8_t *buffer, int length ){
    swizzled_read_or_write( READ, device, buffer, length );
}

void
swizzled_write( void *device, const uint8_t *buffer, int length ){
    swizzled_read_or_write( WRITE, device, buffer, length );
}


static void
swizzled_read_or_write( Direction way, Device d, Base buffer, int length ){
    read_or_write_then_skip( way, d, &buffer, 12, 30 );
    read_or_write_then_skip( way, d, &buffer, 15, 33 );
    read_or_write_then_skip( way, d, &buffer, 17, 22 );
    /* ... etc ... */
    (void) &length;  /* in what way is 'length' supposed to be used? */
}

static void
read_or_write_then_skip( Direction way, Device d, Base *pb, int n, int skip ){
    Base buffer = *pb;
    assert( way == READ || way == WRITE );
    if(  way == READ  )  basic_read( d, (uint8_t *) buffer, n );
    else                basic_write( d,             buffer, n );
    *pb += n;
    skip_forward( d, skip );
}

/****** Finish here ********************************************************/

Back to comp.lang.c | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Generic read/write function - casting vs union Noob <root@127.0.0.1> - 2016-09-22 11:33 +0200
  Re: Generic read/write function - casting vs union Richard Heathfield <rjh@cpax.org.uk> - 2016-09-22 11:07 +0100
    Re: Generic read/write function - casting vs union Noob <root@127.0.0.1> - 2016-09-22 12:36 +0200
      Re: Generic read/write function - casting vs union Richard Heathfield <rjh@cpax.org.uk> - 2016-09-22 14:07 +0100
    Re: Generic read/write function - casting vs union Keith Thompson <kst-u@mib.org> - 2016-09-22 11:48 -0700
    Re: Generic read/write function - casting vs union raltbos@xs4all.nl (Richard Bos) - 2016-09-23 19:42 +0000
  Re: Generic read/write function - casting vs union BartC <bc@freeuk.com> - 2016-09-22 13:11 +0100
    Re: Generic read/write function - casting vs union Noob <root@127.0.0.1> - 2016-09-22 14:46 +0200
      Re: Generic read/write function - casting vs union David Brown <david.brown@hesbynett.no> - 2016-09-22 15:10 +0200
        Re: Generic read/write function - casting vs union Noob <root@127.0.0.1> - 2016-09-22 15:36 +0200
          Re: Generic read/write function - casting vs union Malcolm McLean <malcolm.mclean5@btinternet.com> - 2016-09-22 19:28 -0700
          Re: Generic read/write function - casting vs union David Brown <david.brown@hesbynett.no> - 2016-09-23 09:59 +0200
      Re: Generic read/write function - casting vs union Barry Schwarz <schwarzb@dqel.com> - 2016-09-22 20:11 -0700
      Re: Generic read/write function - casting vs union Richard Damon <Richard@Damon-Family.org> - 2016-09-23 11:13 -0400
  Re: Generic read/write function - casting vs union Tim Rentsch <txr@alumni.caltech.edu> - 2016-09-22 08:35 -0700
    Re: Generic read/write function - casting vs union Noob <root@127.0.0.1> - 2016-09-25 23:17 +0200
    Re: Generic read/write function - casting vs union Noob <root@127.0.0.1> - 2016-09-26 14:54 +0200
      Re: Generic read/write function - casting vs union boon <root@localhost.localdomain> - 2016-09-26 19:47 +0200
      Re: Generic read/write function - casting vs union Ben Bacarisse <ben.usenet@bsb.me.uk> - 2016-09-26 19:23 +0100
      Re: Generic read/write function - casting vs union Tim Rentsch <txr@alumni.caltech.edu> - 2016-09-27 13:10 -0700
      Re: Generic read/write function - casting vs union David Thompson <dave.thompson2@verizon.net> - 2016-10-16 18:08 -0400
  Re: Generic read/write function - casting vs union Anton Shepelev <anton.txt@gmail.com> - 2016-10-17 23:40 +0300
  Re: Generic read/write function - casting vs union albert@cherry.spenarnc.xs4all.nl (Albert van der Horst) - 2016-12-11 15:20 +0100
    Re: Generic read/write function - casting vs union supercat@casperkitty.com - 2016-12-11 09:38 -0800

csiph-web