[Dri-devel] comments for agpgart.h needed
Hi folks, could someone take 1 minute and run through agpgart.h, just taking a look at the structs for the ioctls, and add comments for which "page" fields are ADDRESSES, vs which page fields are INDEXES/page-counts. My head's beginning to spin. I'll narrow it down for ya: typedef struct _agp_segment { off_t pg_start; /* starting page to populate*/ size_t pg_count;/* number of pages */ typedef struct _agp_bind { off_t pg_start; /* starting page to populate*/ typedef struct _agp_allocate { size_t pg_count;/* number of pages */ [Is this really "number of pages", or is it actually "amount of memory"? If really "number of pages", then WHY ISNT IT AN INT?!!] typedef struct _agp_info { size_t pg_used; /* current pages used */ ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] comments for agpgart.h needed
Philip Brown wrote: > Hi folks, > > could someone take 1 minute and run through agpgart.h, > just taking a look at the structs for the ioctls, > and add comments for which "page" fields are ADDRESSES, vs which > page fields are INDEXES/page-counts. My head's beginning to spin. > I'll narrow it down for ya: > > typedef struct _agp_segment { > off_t pg_start; /* starting page to populate*/ > size_t pg_count;/* number of pages */ > > typedef struct _agp_bind { > off_t pg_start; /* starting page to populate*/ > > typedef struct _agp_allocate { > size_t pg_count;/* number of pages */ > [Is this really "number of pages", or is it actually > "amount of memory"? If really "number of pages", > then WHY ISNT IT AN INT?!!] From what is in the code, AFAI understand, this *is* really the number of pages. And 'size_t' is nothing but an 'unsigned int' (in Linux at least...). Here is the relevant excerpt from 'malloc.h' : # undef size_t # define size_t unsigned int It looks like every '*_start' field is an adress, while every '*_count' is a number of pages... a+ -- Nicolas Aspert Signal Processing Laboratory (LTS) Swiss Federal Institute of Technology (EPFL) ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] comments for agpgart.h needed
On Wed, Dec 12, 2001 at 08:15:44AM +0100, Nicolas Aspert wrote: > Philip Brown wrote: > > typedef struct _agp_allocate { > > size_t pg_count;/* number of pages */ > > [Is this really "number of pages", or is it actually > > "amount of memory"? If really "number of pages", > > then WHY ISNT IT AN INT?!!] > > > From what is in the code, AFAI understand, this *is* really the number > of pages. And 'size_t' is nothing but an 'unsigned int' ... Whether it really is an int underneath, is not the point. "size_t" should be used for "sizes". Mostly for BYTE counts of buffers. eg: read(char *,size_t) write(char *,size_t) bcopy (const void *, void *, size_t) "number of pages" is not a "size". It's a count. Hence it should be declared as a plain int. Similarly with the other ones in agpgart. Declaring it as size_t makes it seem like it is the bytecount of all the pages, rather than a number of pages. > It looks like every '*_start' field is an adress, while every '*_count' > is a number of pages... Thanks. I'll doublecheck that, and then try to keep moving on my driver. ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
RE: [Dri-devel] comments for agpgart.h needed
> > > typedef struct _agp_allocate { > > > size_t pg_count;/* number of pages */ > > > [Is this really "number of pages", or is it actually > > > "amount of memory"? If really "number of pages", > > > then WHY ISNT IT AN INT?!!] > > > > > > From what is in the code, AFAI understand, this *is* > really the number > > of pages. And 'size_t' is nothing but an 'unsigned int' ... > > Whether it really is an int underneath, is not the point. > "size_t" should be used for "sizes". > Mostly for BYTE counts of buffers. > eg: read(char *,size_t) > write(char *,size_t) > bcopy (const void *, void *, size_t) > > "number of pages" is not a "size". It's a count. Hence it should be > declared as a plain int. Similarly with the other ones in agpgart. > Declaring it as size_t makes it seem like it is the bytecount > of all the > pages, rather than a number of pages. possibly i am thinking a bit more practical: - the number of pages should never go negative, so why do we need the sign? - there is no reason why the number of pages should get limited to i.e. 2 GB instead of 4 GB on 32 bit machines. - you are right in trying to distinguish number_of_bytes and number_of_elements by meance of different type defines for them. - i am not aware of a better type define - you might want to suggest a new one. Suggestion: typedef unsigned intelcount_t; or #define elcount_t unsigned int But to proove you the opposite: void *calloc(size_t nmemb, size_t size); nmemb => number_of_elements size => number_of_bytes (per element of course) so the usage is already a bit puzzled for other central areas. don't blame the agpgart programmers for introducing this... ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] comments for agpgart.h needed
Alexander Stohr wrote: > > possibly i am thinking a bit more practical: - the number of pages > should never go negative, so why do we need the sign? - there is no > reason why the number of pages should get limited to i.e. 2 GB > instead of 4 GB on 32 bit machines. I think Phil *meant* 'unsigned int' instead of 'int' :-) > - you are right in trying to distinguish number_of_bytes and number_of_elements > > by meance of different type defines for them. - i am not aware of a > better type define - you might want to suggest a new one. > > Suggestion: typedef unsigned intelcount_t; or #define elcount_t > unsigned int > > But to proove you the opposite: void *calloc(size_t nmemb, size_t > size); nmemb => number_of_elements size => number_of_bytes (per > element of course) > > so the usage is already a bit puzzled for other central areas. don't > blame the agpgart programmers for introducing this... > > Yep, that's true... So using 'size_t' for element count seems to be a rather common thing (though 'calloc' is the only example I found so far ;-). The alternative of using another #define might make the code more readable, but we may also stick to the existing version and just add a few comments, s.t. people using it are not puzzled by the signification of the value. BTW, is 'size_t' an 'unsigned int' on every 32-bit platform ? and on the 64-bit ones ? Anyone knows about that ? a+ -- Nicolas Aspert Signal Processing Laboratory (LTS) Swiss Federal Institute of Technology (EPFL) ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] comments for agpgart.h needed
On Wed, Dec 12, 2001 at 01:36:10PM +0100, Alexander Stohr wrote: > - the number of pages should never go negative, so why do we need the sign? > - there is no reason why the number of pages should get limited to i.e. > 2 GB instead of 4 GB on 32 bit machines. But we're talking page count, not byte count. So signed vs unsigned is something like having 8 vs 16 TERRABYTES addressable. Personally, I dont think that should be an issue :-) So allowing signed int for pagecounts, means you can allow -1 as a flag for "uninitialized" or something. Maybe not passing back to the user. But in internal routines that calculate pagecounts, etc. ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] comments for agpgart.h needed
On Wed, Dec 12, 2001 at 01:36:10PM +0100, Alexander Stohr wrote: > > Suggestion: > typedef unsigned intelcount_t; > or > #define elcount_t unsigned int Ack. Don't do that. -- Gareth ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
RE: [Dri-devel] comments for agpgart.h needed
> But we're talking page count, not byte count. So signed vs unsigned is > something like having 8 vs 16 TERRABYTES addressable. > Personally, I dont think that should be an issue :-) well estimated. ;-) consider such a coding: size_t size_of_one_member, total_size_in_bytes; int number_of_members; total_size_in_bytes = size_of_one_member * number_of_members; this might cause some warnings due to the required type intermixing. storing similar objects in compatible types sounds reasonable to me. > So allowing signed int for pagecounts, means you can allow -1 > as a flag for "uninitialized" or something. a special value of zero is sufficient here. > Maybe not passing back to the user. But in internal routines > that calculate pagecounts, etc. with a -1 in that member all your calculations will need extra code for testing this or will give wrong results Regards Alex. PS: to Gareth - i dont do it, unless you give me CVS write permission... ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] comments for agpgart.h needed
On Wed, Dec 12, 2001 at 09:46:23PM +0100, Alexander Stohr wrote: > [phil brown wrote] > > So allowing signed int for pagecounts, means you can allow -1 > > as a flag for "uninitialized" or something. > > a special value of zero is sufficient here. well, yeah, pagecount=0 is fine for an error flag :-) But when you're talking about page INDEX, aka "page offset", thats another thing that should be plain signed int. "off_t" is really only supposed to be used for DISK OFFSETS. It even specifically says it is for disk offsets, if you do grep 'off_t;' /usr/include/sys/types.h under either BSD or Solaris. ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel