[alsa-devel] [PATCH v4 4/6] core: add API header and driver header files

[Takashi Iwai]

At Tue, 13 Dec 2011 19:54:27 +0530,
Vinod Koul wrote:
> 
> On Tue, 2011-12-13 at 15:04 +0100, Takashi Iwai wrote:
> > At Tue, 13 Dec 2011 19:21:30 +0530,
> > Nallasellan, Singaravelan wrote:
> > > 
> > > > > > +	size_t buffer_size;
> > > > > > +	size_t fragment_size;
> > > > > Can we define buffer_size and fragment_size as unsigned items?
> > > > 
> > > > size_t is unsigned.
> > > > 
> > > > But, it needs consideration whether size_t is the best choice, since size_t is basically
> > > > a long, thus its size is different between 32bit and 64bit architectures.  This may lead
> > > > to mess in many cases when you think of overlapping.
> > > > 
> > > Sorry for my mess on size_t.
> > > I think the assumption here is 32 bits. Should u32 be fine here?
> > 
> > This depends pretty much on the demand and the implementation.
> > Unless a buffer over 4GB is needed, u32 should be OK for buffer_size,
> > etc.  (Or, in this case, unsigned int would be fine, too.  It's not
> > necessarily be limited to 32bit.  u32 or such is used usually for the
> > type that must be 32bit.)
> I don't think someone would need buffer of 4GB :)
> 
> > But, the questions remain for hw_pointer, app_pointer, bytes_*
> > fields.  Are these supposed to be 32bit or more?  This question is
> > more important than the buffer size.  The buffer size would be rarely
> > over 32bit.  But, if hw_pointer or such represents the accumulated
> > position like ALSA PCM does, you'll face the overlapping problem.
> > 32bit limit can be reached easily in the real use case.
> the pointer are offsets within the ring buffer.

OK, then it's no problem to assume 32bit is enough.

> there are cumulative counters which will get overlapped after the
> size_t/u32 value.

Which field specifically?  The cumulative counter has been a problem.

> > When this changes between 32bit and 64bit, think twice about the 32bit
> > compat-layer on 64bit arch (and the corresponding user-space
> > implementation)...
> Is it a fair assumption that userspace compiled for 32 bit should work
> with 32 bit kernel and same for 64 bit, or does the ABI mandate it be
> consistent across 32 and 64 bits.

The 32bit user-space must work with 64bit kernel.  It means, it'd be
much easier if ABI definition is compatible between 32 and 64bit.

That is, using a long type in the ABI definition should be definitely
avoided.  We had long fields in ALSA PCM field definitions, and this
caused lots of really nasty problems.  So, I tell you -- don't do it
again.  Learn from the history :)

Of course, this is about the ABI definition.  But, the internal
structure (types, etc) usually follows the ABI requirement.  So I
stated the questions here.

> In the latter case we should change
> the ones in kernel ABI to consistent size of u32/u64. IMO u32 should be
> fine, please let me know if anyone thinks otherwise.
> In former size_t should be fine :)


thanks,

Takashi