pdf-devel
[Top][All Lists]
Advanced

[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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]