[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
