tidy'ing up cz_hwmgr.c

Alex Deucher alexdeucher at gmail.com
Thu Aug 18 15:33:41 UTC 2016


The problem we ran into was when we had a struct like this:

struct table {
   uint16_t size;
   struct element elements[0];
};

and then we would try and index the array:

for (i = 0; i < table->size; i++) {
  element = &table->elements[i];
}

element ended up off in the weeds.  The only thing that seems to make some
versions of gcc happy was pointer arithmetic.  E.g.,

element = (struct element *)((char *)&table->elements[0] + (sizeof(struct
element) * i));

Alex

On Thu, Aug 18, 2016 at 11:21 AM, StDenis, Tom <Tom.StDenis at amd.com> wrote:

> Any modern GCC should support [0] at the tail of a struct.  This came up
> because when I was reading the code I saw they allocated 7 slots (plus the
> size of the struct) but then fill 8 slots.  It's just weird [image: 😊]
>
>
> Using [0] in the struct and allocating for 8 entries makes more sense and
> is clearer to read.
>
>
> Tom
>
>
> ------------------------------
> *From:* Christian König <deathsimple at vodafone.de>
> *Sent:* Thursday, August 18, 2016 11:17
> *To:* StDenis, Tom; amd-gfx list
> *Subject:* Re: tidy'ing up cz_hwmgr.c
>
>
> Has a [1] array at the tail which is then kzalloc'ed with N-1 entries.
> Shouldn't that just be a [0] with N entries allocated for clarity?
>
> Actually the starting address of a dynamic array should be manually
> calculated instead of using [1] or [0].
>
> We had tons of problems with that because some gcc versions get this wrong
> and the atombios code used this as well.
>
> Alex how did we resolved such issues?
>
> Regards,
> Christian.
>
> Am 18.08.2016 um 16:26 schrieb StDenis, Tom:
>
> Tidying up cz_hwmgr.c I noted a couple of things but first is
>
>
> static bool cz_dpm_check_smu_features(struct pp_hwmgr *hwmgr,
> unsigned long check_feature);
>
> Which will return "true" if the smu call fails *or* the feature is set.
>
> The structure
>
> struct phm_clock_voltage_dependency_table;
>
> Has a [1] array at the tail which is then kzalloc'ed with N-1 entries.
> Shouldn't that just be a [0] with N entries allocated for clarity?
>
> Tom
>
>
>
> _______________________________________________
> amd-gfx mailing listamd-gfx at lists.freedesktop.orghttps://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160818/7a632ff7/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OutlookEmoji-😊.png
Type: image/png
Size: 488 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160818/7a632ff7/attachment-0001.png>


More information about the amd-gfx mailing list