andrewrk / andrewrk/libsoundio

Alignment of pointers vis-a-vis casting

オープン
#107 コメント 6 件 リアクション 1 件 担当者 0 名 GitHub で見る
enhancement
主要言語
C
スター
2.1k
フォーク
254
PR マージ指標
30日以内にマージされた PR はありません

説明

Building with `-Wcast-align` yields a few warnings, like this one:

```
libsoundio/example/sio_sine.c:49:19: warning: cast from 'char *' to 'double *' increases required alignment from 1 to 8 [-Wcast-align]
```

This results from the existing practice of casting `(struct SoundIoChannelArea).ptr` (which is a `char *`) to whatever type the sample happens to be, which is usually larger than a `char`.

Of course, the `.ptr` field should always be aligned appropriately for the applicable sample type, but the compiler has no way of knowing that, and it's not good practice to make mental exceptions to compiler warnings.

I'd like to suggest an alternate approach for `SoundIoChannelArea`. This would have to be slated for a future major version of the API, of course, but I think it would be cleaner and a good way to go:

```
struct SoundIoChannelArea {
union {
int16_t *s16ne;
int32_t *s32ne;
float *float32ne;
double *float64ne;
} ptr;
int step;
}
```

Then instead of doing something like

```
*(int16_t *)areas[channel].ptr = sample;
areas[channel].ptr += areas[channel].step;
```

which annoys the compiler, you can do

```
*areas[channel].ptr.s16ne = sample;
/* .step is now in terms of samples, not bytes */
areas[channel].ptr.s16ne += areas[channel].step;
```

which leaves it happy as a clam.

(This wouldn't only apply to `SoundIoChannelArea`; there's also `soundio_ring_buffer_{read,write}_ptr()` and the like. But this would be the main target.)

As for lower-hanging fruit, there's one place that is an easy fix. In `src/alsa.c`, there are a couple of these:

```
osa->chmap = (snd_pcm_chmap_t *)ALLOCATE(char, osa->chmap_size);
```

`ALLOCATE()` expands to a call to `calloc()` and a cast to the type (with star) passed as the first argument. Alas, however, the compiler then sees a cast from `char *` to `snd_pcm_chmap_t *` and complains---even though the address just came from a system memory allocation, which should be aligned for everything.

Here's one way to straighten that out:

```
#define ALLOCATE_NOCAST(Type, count) (calloc(count, sizeof(Type)))

#define ALLOCATE(Type, count) ((Type*)ALLOCATE_NOCAST(Type, count))
```

Then you use `ALLOCATE_NOCAST()` in the above instance.

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。