Support multiple NoteData within a NoteSegment #87
Conversation
ELFSharp/ELF/Sections/NoteData.cs
Outdated
| { | ||
| internal string Name { get; private set; } | ||
| public const ulong NOTE_DATA_HEADER_SIZE = 12; // name size + description size + field |
There was a problem hiding this comment.
Naming convention for constants in the project is PascalCase.
ELFSharp/ELF/Segments/INoteData.cs
Outdated
| /// <summary> | ||
| /// Owner of the note. | ||
| /// </summary> | ||
| string Name { get;} |
There was a problem hiding this comment.
| string Name { get;} | |
| string Name { get; } |
ELFSharp/ELF/Segments/INoteData.cs
Outdated
| /// <summary> | ||
| /// Owner of the note. | ||
| /// </summary> | ||
| string Name { get;} |
There was a problem hiding this comment.
| string Name { get;} | |
| string Name { get; } |
ELFSharp/ELF/Sections/NoteData.cs
Outdated
| internal byte[] Description { get; private set; } | ||
| public string Name { get; private set; } | ||
|
|
||
| public byte[] Description { get; private set; } |
There was a problem hiding this comment.
As we now go public, I'd like to go with something immutable (I know that we have it in NoteSegment, but this was a mistake. As ImmutableArray is not available in .NET Standard 2.0, I think that ReadOnlyCollection would be the best choice.
ELFSharp/ELF/Sections/NoteData.cs
Outdated
| internal byte[] Description { get; private set; } | ||
| public string Name { get; private set; } | ||
|
|
||
| public byte[] Description { get; private set; } |
There was a problem hiding this comment.
As we now go public, I'd like to go with something immutable (I know that we have it in NoteSegment, but this was a mistake. As ImmutableArray is not available in .NET Standard 2.0, I think that ReadOnlyCollection would be the best choice.
ELFSharp/ELF/Segments/NoteSegment.cs
Outdated
| using ELFSharp.ELF.Sections; | ||
| using ELFSharp.Utilities; | ||
|
|
||
| namespace ELFSharp.ELF.Segments | ||
| { | ||
| public sealed class NoteSegment<T> : Segment<T>, INoteSegment | ||
| { | ||
| private List<NoteData> mNotes = new List<NoteData>(); |
There was a problem hiding this comment.
In this project private fields should be located at the end of the class. Also, don't prefix private fields with m or anything else, so just notes.
ELFSharp/ELF/Segments/NoteSegment.cs
Outdated
| using ELFSharp.ELF.Sections; | ||
| using ELFSharp.Utilities; | ||
|
|
||
| namespace ELFSharp.ELF.Segments | ||
| { | ||
| public sealed class NoteSegment<T> : Segment<T>, INoteSegment | ||
| { | ||
| private List<NoteData> mNotes = new List<NoteData>(); |
There was a problem hiding this comment.
In this project private fields should be located at the end of the class. Also, don't prefix private fields with m or anything else, so just notes.
| } | ||
| else | ||
| { | ||
| // File is damaged |
There was a problem hiding this comment.
I'd go with throwing exception here, unless you know that non-conforming files are frequent. Otherwise we'll wait for such an issue with accepting structure like here.
| } | ||
| else | ||
| { | ||
| // File is damaged |
There was a problem hiding this comment.
I'd go with throwing here, unless you know that non-conforming files are frequent. Otherwise we'll wait for such an issue with accepting structure like here.
There was a problem hiding this comment.
I can add an exception here. My reasoning for not doing it was that "if there are non-confirming files", that means that the change would be breaking behavior for backwards compatibility.
|
Requested changes commited in b61c3cb |
ELFSharp/ELF/Sections/NoteData.cs
Outdated
| { | ||
| internal string Name { get; private set; } | ||
| public const ulong NOTE_DATA_HEADER_SIZE = 12; // name size + description size + field |
There was a problem hiding this comment.
Naming convention for constants in the project is PascalCase.
Added support for multiple NoteData within a single NoteSegment, as is common in ELF core dumps: