linux-msm / qdl

BSD 3-Clause "New" or "Revised" License
196 stars 76 forks source link

program: fail correctly on unrecognized tags #57

Closed z3ntu closed 1 month ago

z3ntu commented 2 months ago

Previously the message would be printed that the tag gets ignored but in reality the 'errors' variable will still be initialized as -EINVAL so the if below would return an error.

[PROGRAM] unrecognized tag "zeroout", ignoring
qdl: program_load rawprogram0.xml failed

Fix this.

lumag commented 2 months ago

Should qdl instead fail on unrecognized tags?

konradybcio commented 2 months ago

hm, on a second thought some unsupported/unrecognized tags may be "important".. perhaps it could use a --flag?

z3ntu commented 2 months ago

This is for "zeroout" and "information" tags. The instructions we got from the ODM was to just remove them from the provided .xml file for flashing with Qualcomm's tooling (fh_loader), so I also wouldn't be sad to just fail on unrecognized tags.

But the message is there right now and the code doesn't do what the print is saying, that's why I opened this ;)

lumag commented 2 months ago

@z3ntu could you please ignore just those two tags and make qdl fail on other uknown tags?

z3ntu commented 2 months ago

I've edited my previous comment for clarity, I think I wrote it a bit unclear.

To be clear, I have no idea what tool would handle "zeroout" (I'm guessing this tag is meant to just write zeroes to the partition?). <information sounds safer to ignore, I'm okay with ignoring that one.

But I can look into making this fail on purpose on unsupported tags, should be only a few lines change at max anyways.

z3ntu commented 2 months ago

Not sure what I can "handle" there, also not sure we should just ignore them.

These are the tags from rawprogram0.xml that our partner suggests to just remove from the file for QFIL flashing.

<zeroout SECTOR_SIZE_IN_BYTES="4096" factory_erase_only="false" label="userdata" num_partition_sectors="0" physical_partition_number="0" start_sector="3790029"/>
<zeroout SECTOR_SIZE_IN_BYTES="4096" factory_erase_only="false" label="metadata" num_partition_sectors="7648" physical_partition_number="0" start_sector="8736"/>
<information FCM_VERSION="1" STORAGE_TYPE="ufs" TABLE_VERSION="2"/>

(Similar <information> tags in other rawprogram*.xml files are present also)

I think I'll just rework this PR to just properly disallow unrecognized tags and leave the rest for someone else to handle.

lumag commented 2 months ago

Sounds good.

z3ntu commented 1 month ago

Implemented the change. Also added a commit on top to clean up some error handling there. Let me know what you think.