[LU-4629] Issues found by static analysis tools Created: 13/Feb/14 Updated: 30/Aug/23 Resolved: 20/Jun/14 |
|
| Status: | Resolved |
| Project: | Lustre |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | Lustre 2.6.0 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Dmitry Eremin (Inactive) | Assignee: | Dmitry Eremin (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | kw | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Severity: | 3 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Rank (Obsolete): | 12663 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
This is a common ticket for all issues found by static analysis tools. |
| Comments |
| Comment by Dmitry Eremin (Inactive) [ 21/Feb/14 ] |
|
Reviewed patches can be observed by http://review.whamcloud.com/#/q/message:LU-4629,n,z |
| Comment by James A Simmons [ 06/Mar/14 ] |
|
I noticed several of these patches handle code paths that previously ignored returned NULL pointers. What I find confusing is when is LASSERT a proper solution versus just handling the error condition. There doesn't seem to be any clear standard in this practice in the Lustre code. |
| Comment by Mikhail Pershin [ 22/Apr/14 ] |
|
James, there are two types of 'capsules' - server and client. Client 'capsule' is request buffers packed on client, server - packed reply buffers on server. So the main rule is to don't assert on wire data, so server must checks all clients buffers and return -EPROTO on them in case of error, the client must do the same for server buffers. Meanwhile server may assert on own just packed reply buffers because this means internal logic error in code, e.g. buffer wasn't properly packed. Just an error code is not working well in such cases because may be not noticed for a long time, while assertion will occur during new code development/testing, well, it should at least. So we are talking about two different cases - checking the incoming data through the wire (protocol correctness) and internal checks for code correctness, e.g. buffer must exists after successful allocation. |
| Comment by James A Simmons [ 01/May/14 ] |
|
Thank you Mikhail for the explanation. I have noticed several patches deal with strncpy issues. Why not use the kernels internal kstr* functions instead. This would seem a better way to go. We just have to implement wrappers for the non linux platforms. |
| Comment by Dmitry Eremin (Inactive) [ 05/May/14 ] |
|
In most places buffer already allocated and usage of kstrndup() is not appropriate. Also I don't think that allocation many little chunks is a good idea. I prefer to allocate big chunk once instead of many small. |
| Comment by Dmitry Eremin (Inactive) [ 09/Jun/14 ] |
|
Add more patches: http://review.whamcloud.com/#/q/message:LU-4629,n,z |
| Comment by Peter Jones [ 20/Jun/14 ] |
|
Remaining open patches landed for 2.6. Any future such issues will be tracked under new tickets linked to this one |
| Comment by Ian Costello [ 21/Oct/14 ] |
|
Hi Peter, Oleg Could we get this one bug completed with the review process and committed to 2.5? Thanks, |