Thanks for comments!
2013/4/7 Paul Menzel <paulepanter@xxxxxxxxxxxxxxxxxxxxx>:
> Am Sonntag, den 07.04.2013, 12:52 +0200 schrieb Rafał Miłecki:
>> +struct cea_sad *drm_edid_to_sad(struct edid *edid)
>> +{
>> + struct cea_sad *sads = NULL;
>
> No need to set it NULL, as it gets assigned later on? Looks like in the
> end there is a corner case, where nothing gets assigned in the
> `for_each_cea_db` loop. So the compiler is going to complain.
Right, I can drop that. Curious, my gcc didn't complain.
>> + if (cea_revision(cea) < 3) {
>> + DRM_DEBUG_KMS("SAD: wrong CEA revision\n");
>> + return NULL;
>> + }
>> +
>> + if (cea_db_offsets(cea, &start, &end)) {
>> + DRM_DEBUG_KMS("SAD: invalid data block offsets\n");
>> + return NULL;
>> + }
>
> Could the above be put in some helper too:
> `is_cea_valid_for_version(cea, version)`?
Not sure if it's worth a helper. But maybe I'll go and just use a single check?
if (cea_revision(cea) < 3 || cea_db_offsets(...))
>> + for_each_cea_db(cea, i, start, end) {
>> + db = &cea[i];
>> + dbl = cea_db_payload_len(db);
>> +
>> + if (cea_db_tag(db) == AUDIO_BLOCK) {
>> + int count = dbl / 3; /* SAD is 3B */
>> + int j;
>> +
>> + sads = kzalloc(count + 1 * sizeof(*sads), GFP_KERNEL);
>> + if (!sads)
>
> Add an error too? »kzallac failed« or something like this?
Allocation failures are aloud enough on the lower level. I was
corrected few times already to don't put warnings when alloc fails. If
you want me to point to the discussions, I'll have to search though.
>> +#define SAD_FORMAT_LPCM 0x01
>> +#define SAD_FORMAT_AC3 0x02
>
> At least in my MUA indentation looks different.
It's just a matter of .patch format. Patch file has "+" sign at a
beginning of line and \t can look different when viewing patch. Please
apply a patch and check drm_edid.h then - it'll be OK.
--
Rafał
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
