Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[refactoring] Optimisation for processing chipid files #1234

Merged

Conversation

Sylensky
Copy link

@Sylensky Sylensky commented Mar 8, 2022

Hi everyone,

during my work i made some changes to the stlink library which i thought of would benefit the overall project.
These changes include optimizations in the processing of chipfiles and in a seperate PR the addition of option byte descriptions.

  • printing processed chipfiles only when it makes sense (not in st-info --version)
  • moved the dump chip printout from stderr to stdout
  • the chipfile read buffer was quite large for a rather small file and since its processed line per line it doesn't make sense to define the buffer that large. (maybe i made it too small when someone adds a rather big description in the chipfile?)
  • sscanf already looks for spaces and seperates them so there was no need to explicitly look for spaces in my opinion
  • the filedescriptor wasn't closed after the file has been processed. - fixed that

Lucas Sinn added 2 commits March 8, 2022 11:22
'moved chip dump to stdout instead of stderr'
'closing open fd after file is fully read'
'reduced file read buffer size'
Copy link
Collaborator

@gszy gszy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Sylensky, thanks for your contribution. I’ve left a few comments in the changes.

src/stlink-lib/chipid.c Outdated Show resolved Hide resolved
src/stlink-lib/chipid.c Show resolved Hide resolved
src/stlink-lib/chipid.c Outdated Show resolved Hide resolved
src/st-info/info.c Outdated Show resolved Hide resolved
@gszy
Copy link
Collaborator

gszy commented Mar 20, 2022

I’m sorry for the delay, I hope I’ll be able to review this PR next week.

@Nightwalker-87
Copy link
Member

Yes, there is nothing else I can report from my side actually... 😔
@Sylensky Please stay tuned - it will remain on our ToDo list.

@gszy
Copy link
Collaborator

gszy commented Mar 26, 2022

Well, no luck, I won’t have enough time. Just one more thing I’ve spotted: the new small buffer size may cause issues on lines having long comments.

@Sylensky
Copy link
Author

I thought about that aswell. Making it double should totally be enough. Or do you see a case where such long comments might still be useful?

@gszy
Copy link
Collaborator

gszy commented Mar 31, 2022

I’m not sure about its usefulness, but FWIW here F76x_F77x.chip has quite a long line:

option_base 0x1fff0000 // STM32_F7_OPTION_BYTES_BASE /* Used for reading back option bytes, writing uses FLASH_F7_OPTCR and FLASH_F7_OPTCR1 */

@Sylensky
Copy link
Author

Sylensky commented Apr 1, 2022

I checked the other files and the F76x_F77x.chip has the longest line so far. Choosing a buffer size of 256 should be in my opinion enough even if there is going to be more description on a line.

@Nightwalker-87 Nightwalker-87 removed the request for review from gszy May 14, 2022 14:40
@Nightwalker-87 Nightwalker-87 changed the title chipid optimization Optimisation for processing chipid files May 14, 2022
@Nightwalker-87 Nightwalker-87 merged commit 6870c82 into stlink-org:develop May 14, 2022
@Nightwalker-87 Nightwalker-87 changed the title Optimisation for processing chipid files [refactoring] Optimisation for processing chipid files Aug 26, 2022
@stlink-org stlink-org locked as resolved and limited conversation to collaborators Aug 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants