serge1 / ELFIO

ELFIO - ELF (Executable and Linkable Format) reader and producer implemented as a header only C++ library
http://serge1.github.io/ELFIO
MIT License
706 stars 152 forks source link

Add insert_data() to section #114

Closed covanam closed 1 year ago

covanam commented 1 year ago

I'm working on a ELF patching library, and I find myself with the need for inserting some data in the middle of the section. I have to use set_data(), but it would be nice if there is built-in support from ELFIO.

This implementation is largely based on existing append_data() functions.

serge1 commented 1 year ago

Thank you for your submission!

insert_data() and append_data() have practically the same implementation. Would not it be nicer to add 'position' argument to append_data() as an optional argument defaulting to 'append at the end'?

covanam commented 1 year ago

Thank you for your submission!

insert_data() and append_data() have practically the same implementation. Would not it be nicer to add 'position' argument to append_data() as an optional argument defaulting to 'append at the end'?

Thanks for the review.

Yes I agree that it makes sense to merge the implementations together. However, in my opinion, adding argument position to append_data is not a good idea, because the function name gives the impression that data will be put at the end, and it may be confusing that an append function can also insert data in the middle.

Perhaps it is better to keep both append_data and insert_data. We can have append_data calling insert_data, with position argument being set to the end. What do you think?

serge1 commented 1 year ago

We can have append_data calling insert_data, with position argument being set to the end. What do you think?

Yes, I think it will be better.

Now, are you going to implement such solution or are expecting it from me. The effort is not big, so, I don't care much.

covanam commented 1 year ago

I have pushed a new commit for this.

serge1 commented 1 year ago

Accepted. Thank you very much!