[asterisk-dev] [Code Review] Eliminate memmove from pbx_spool implementation

[Tilghman Lesher]

> On 2010-09-07 17:30:00, Kevin Fleming wrote:
> > /branches/1.8/pbx/pbx_spool.c, line 525
> > <https://reviewboard.asterisk.org/r/912/diff/1/?file=12430#file12430line525>
> >
> >     You *might* be able to use aligned(alignof(struct inotify_event)) here, but I'm not quite sure which versions of GCC support the 'alignof' keyword. It was created for this very reason, though :-)

I'm showing that 'alignof' may be part of a C++ standard, but it's not clear that it's quite there yet.  Given that the kernel ABI of inotify(7) has stabilized, it's unlikely that the first element type will change from an int, so this is probably close enough.  If there's word that 'alignof' is now standardized (as opposed to the competing 'alignment_of'), then that would probably be the way to go.


> On 2010-09-07 17:30:00, Kevin Fleming wrote:
> > /branches/1.8/pbx/pbx_spool.c, line 597
> > <https://reviewboard.asterisk.org/r/912/diff/1/?file=12430#file12430line597>
> >
> >     Might as well use 'size_t' here, if it doesn't result in compiler warnings when included in the operations involving 'res'.

I'm showing that read(2) returns an ssize_t, so I'll change len to that same type, as well.


> On 2010-09-07 17:30:00, Kevin Fleming wrote:
> > /branches/1.8/pbx/pbx_spool.c, line 601
> > <https://reviewboard.asterisk.org/r/912/diff/1/?file=12430#file12430line601>
> >
> >     This won't do what you expect; it will advance the 'iev' pointer by the size of 'len' inotify_events (len * sizeof(*iev)). To be able to advance it by a specific number of bytes, you'll have to either cast it to a 'char *' before adding to it, or put it in a union with a 'char *' that overlays it and add the offset to the 'char *' instead. The former method is used fairly often in Asterisk code, so it should be fine.

Changed.


- Tilghman


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/912/#review2693
-----------------------------------------------------------


On 2010-09-07 17:21:52, Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/912/
> -----------------------------------------------------------
> 
> (Updated 2010-09-07 17:21:52)
> 
> 
> Review request for Asterisk Developers and Kevin Fleming.
> 
> 
> Summary
> -------
> 
> As requested by kpfleming
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/pbx/pbx_spool.c 285386 
> 
> Diff: https://reviewboard.asterisk.org/r/912/diff
> 
> 
> Testing
> -------
> 
> Compile testing only
> 
> 
> Thanks,
> 
> Tilghman
> 
>