[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [pdf-devel] Changes in the Time module API
From: |
jemarch |
Subject: |
Re: [pdf-devel] Changes in the Time module API |
Date: |
Tue, 29 Apr 2008 20:35:54 +0200 |
User-agent: |
Wanderlust/2.14.0 (Africa) SEMI/1.14.6 (Maruoka) FLIM/1.14.8 (Shijō) APEL/10.6 Emacs/23.0.60 (powerpc-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) |
Hi.
I revised the Time module API and I propose the changes attached to the
email. Basically:
- Set as `const' those functions parameters that won't be modified in
the function call, to easily detect which are input parameters and which
are output parameters.
It is a good practice. I think that would be good to apply it in the
existing modules too. I will open a low-priority task for this.
- Calendar type stores month/day... in 32bit variables. Probably 8 bits
are enough for these fields (not for `year', which remains in
32bit).
To use 32bit words to store these values would improve the efficiency
of the library, if running in usual 32bit machines. My feeling is that
we should use 32bit integers even if we are wasting space. It is not a
strong feeling anyway.
- Changed `pdf_i32_t' name to `pdf_32_t', as defined in pdf_types.h.
BTW, which one is better? I prefer `pdf_32_t', but I could change it.
`pdf_u32_t' remains as is.
I much prefer the `pdf_i32_t' name, since it makes explicit that the
stored value is a (signed) integer.
- Those functions filling in output `struct pdf_time_cal_span_s' and
`struct pdf_time_cal_s' variables should manage pointers to the
structures (passing by reference the structure in order to be able to
modify its contents).
Yes. It is a bug in the API.
- Changed the name of the pdf_time_t variable passed to the functions
from `time' to `object', to avoid using the same name as the `time()'
function.
Hm? `time' is more explicit. Being an actual parameter, where is the
confusion with time()?
- The function `pdf_time_span_to_secs' was returning a 32bit value to
represent the time span in seconds. As the time span is stored as a
64bit value internally, it should return a pdf_64_t.
Another bug in the API :)
Thanks aleks.
--
Jose E. Marchesi <address@hidden>
<address@hidden>
GNU Spain http://es.gnu.org
GNU Project http://www.gnu.org