On Mon, Aug 21, 2017 at 8:36 AM, Gert Wollny <gw.foss...@gmail.com> wrote: > Am Sonntag, den 20.08.2017, 15:48 +0200 schrieb Marek Olšák: >> The patch seems OK to me. > > Hmm, > > you're right in that I was wrong about "program", I still need learn to > think in patches. > > However, when I look at si_delete_compute_state where > si_compute_reference is called: > > >>+ si_compute_reference(&program, NULL); >>+} > ... >>+static inline void >>+si_compute_reference(struct si_compute **dst, struct si_compute *src) >>+{ >>+ if (pipe_reference(&(*dst)->reference, &src->reference)) >>+ si_destroy_compute(*dst); > > then src will be a NULL pointer and de-referencing it is most likely > already undefined behavior. > > Anyway, in the end the address of a member is taken (i.e. > src+offsetof(src, reference), and passed as a pointer, and this pointer > is later de-referenced in > > pipe_reference_described > > if it is not 0. > > For now this runs through because "reference" is the first element in > the si_compute struct, and hence its offset is 0 and &src->reference > will also be 0, but the moment someone decides to reorder si_compute > this may no longer the case and the code would break. > > At the least it should be commented that the element reference within > the structure si_compute must always be at offset 0.
You're right. All pipe_reference declarations should always be first. And that's why they are always first. It's an unwritten rule in Gallium. Marek _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev