On Thu, 2007-10-25 at 08:58 -0300, Gustavo Sverzut Barbieri wrote:
> On 10/25/07, Brett Nash <[EMAIL PROTECTED]> wrote:
> > > > struct _Ecore_Exe_Event_Del /** Process exit event */
> > > > {
> > > > - pid_t pid; /**< The process ID of the process that exited */
> > > > - int exit_code; /**< The exit code of the process */
> > > > - Ecore_Exe *exe; /**< The handle to the exited process, or NULL if
> > > > not found */
> > > > - int exit_signal; /** < The signal that caused the process to
> > > > exit */
> > > > - char exited : 1; /** < set to 1 if the process exited of
> > > > its own accord */
> > > > - char signalled : 1; /** < set to 1 id the process exited due
> > > > to uncaught signal */
> > > > - void *ext_data; /**< Extension data - not used */
> > > > - siginfo_t data; /**< Signal info */
> > > > + pid_t pid; /**< The process ID of the process that exited */
> > > > + int exit_code; /**< The exit code of the process */
> > > > + Ecore_Exe *exe; /**< The handle to the exited process, or NULL
> > > > if not found */
> > > > + int exit_signal; /** < The signal that caused the process
> > > > to exit */
> > > > + unsigned char exited : 1; /** < set to 1 if the process exited
> > > > of its own accord */
> > > > + unsigned char signalled : 1; /** < set to 1 id the process exited
> > > > due to uncaught signal */
> > > > + void *ext_data; /**< Extension data - not used */
> > > > + siginfo_t data; /**< Signal info */
> > > > };
> >
> > In any case, I'd rather just do the minimum change to remove the
> > warning/unportable-declaration - which is add unsigned.
>
> I think Vincent was wondering about my last commit to ETK, that I
> tried to remove struct holes with information provided by pahole. In
> your case it will contain these holes since structs will be aligned,
> and things like [char][pointer] will be laid out effectively as
> [long][pointer], same for int, long, ... in the place of [pointer].
> For better use of memory, you should try to pack-a-hole (pahole
> tool is meant for that) and even better: keep memory that is used
> together within the same cacheline (pahole helps here to).
I seem to have unintentionally opened a can of worms here. Just to be
clear they were meant minimal patches - adding unsigned to 1-bit fields.
Additional changes were to make the code line up in the same way most of
the structures are aligned.
The reason I went after the bit fields is because they are totally
undefined in C - the valid range is exactly the value 0. They may not
even store a second value successfully (more specifically they may store
0 and -0, which both compare true to 0).
If anyone wants to sort the structures for packing or similar, feel
free, I'm happy to apply sane patches, and deal with carsten beating me
up later ;-)
Currently I'm trying to get the warnings and errors from the static
analysis tools we use under control, as well as make the code clean on
64 bit systems. This is both for the commercial version we support and
upstream.
> All in all is usually better to keep these bitfield as unsigned char
> (or int, or short,...) at the end of the struct.
And I'll generally agree with this. Although there is a counter
argument if you really want to optimise cache-line friendliness ;-)
(eg lists should always be {data, next, prev} not {next, prev, data}).
However that's way out of scope at this time ;-)
Regards,
nash
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
