search

accellera-official / systemc

SystemC Reference Implementation
https://systemc.org/overview/systemc/
Apache License 2.0
451 stars 145 forks source link

pkt_switch example suggestions #15

Closed HZ-HZ closed 2 years ago

HZ-HZ commented 3 years ago

We found several possible problems in the pkt_switch routine under the example folder. Here are my suggestions. First, in the fifo.cpp file, the pkt_out method lacks the logic to reset the "full", which results in no more data being read after the fifo is full. Second, in Line 204, 210, 216 in the switch.cpp file, the judgment condition of R1.free, R2.free, R3.free is wrong, which causes some data packets to be lost too early.

markfoodyburton commented 2 years ago

Hi, thanks for your contribution, would you mind adding a brief description to the commit message and a signed-of-by line, with your affiliation.

For your contribution to be applicable for consideration by the Accellera working groups, you need to sign-off the Accellera Developer's Certificate of Origin as described in the CONTRIBUTING file.

Thanks

HZ-HZ commented 2 years ago

Thanks for the kindly reminder. But I'm not very familiar with Github. I don't know how to add the signed-off-by line. Should I Close this Pull Request and Create a new Pull Request or should I use a local IDE to do it? Thank you very much again.

markfoodyburton commented 2 years ago

You just need to amend the commit. Thanks

HZ-HZ commented 2 years ago

I amended my commit to add a signed-off-by line. Did I do it correctly? Thank you.

aut0 commented 2 years ago

Patch itself looks correct, but commit messages are missing title line and the commit message itself is very uninformative and too short. I would recommend adding title line and elaborate more clearly what the fix does.

HZ-HZ commented 2 years ago

Hi, I just added the title line and added more details to the commit message. Does it look appropriate now? Thank you.