Skip to content

[sonic_eeprom] If reading from what appears to be a corrupt cache file, delete file and read directly from EEPROM#10

Merged
lguohan merged 2 commits intosonic-net:masterfrom
jleveque:decode_syseeprom_corrupt_cache_file_fix
Aug 10, 2018
Merged

[sonic_eeprom] If reading from what appears to be a corrupt cache file, delete file and read directly from EEPROM#10
lguohan merged 2 commits intosonic-net:masterfrom
jleveque:decode_syseeprom_corrupt_cache_file_fix

Conversation

@jleveque
Copy link
Copy Markdown
Contributor

@jleveque jleveque commented Aug 9, 2018

Before this PR, if a corrupt cache file exists, decode-syseeprom will fail indefinitely until the file is manually deleted.

…e, delete file and read directly from EEPROM
@jleveque jleveque self-assigned this Aug 9, 2018
@jleveque jleveque requested review from lguohan and qiluo-msft August 9, 2018 21:49
os.remove(self.cache_name)
self.cache_update_needed = True
F.close()
return self.read_eeprom_bytes(byteCount, offset)
Copy link
Copy Markdown

@qiluo-msft qiluo-msft Aug 9, 2018

Choose a reason for hiding this comment

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

read_eeprom_bytes [](start = 28, length = 17)

The code may achieve the goal. However, the recursion logic is not intuitive to solve a simple if-else problem.

I will not block this PR. Strongly suggest refactor the code. #Closed

@lguohan lguohan merged commit 664d6cc into sonic-net:master Aug 10, 2018
@jleveque jleveque deleted the decode_syseeprom_corrupt_cache_file_fix branch August 10, 2018 17:17
lguohan pushed a commit that referenced this pull request Aug 20, 2018
…e, delete file and read directly from EEPROM (#10)

* [sonic_eeprom] If reading from what appears to be a corrupt cache file, delete file and read directly from EEPROM

* Refactor to eliminate recursion
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-platform-common that referenced this pull request Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants