Ticket #115 (new enhancement)

Opened 1 year ago

Last modified 1 year ago

Add typedefs

Reported by: pfritz Owned by: pygi
Priority: trivial Milestone:
Component: libburn Version:
Keywords: Cc:

Description

Here is a patch to add type definitions for the structures used in libburn(.h). It should be binary and source compatible to older version. The only problem that could arise is if someone has typedef these definitions in his app, too. The format is the same like in libisofs. If you like it, I can patch the *.c-files, too. I also want to add a @since in the doxy comment. Which version should stand there?

I hope you like it.

Attachments

typedef.diff (34.7 kB) - added by pfritz 1 year ago.
first patch

Change History

Changed 1 year ago by pfritz

first patch

  Changed 1 year ago by pygi

Personally, I support this suggestion. In regards with version that should stand there, it should be 0.4.4, as that will probably be our next stable release.

follow-up: ↓ 5   Changed 1 year ago by scdbackup

On the risk to appear stubborn: where is the benefit in the types ?

Any change has a benefit / risk ratio. The risk may be very small. But with 0 benefit it still has 0 ratio.


General considerations before you start to work in libburn:

You need to teach me how to process your patches. Needed is a trustworthy shell procedure, step by step for elderly people.

libburn is the next code that will be replaced in this overall project. I began with the design of the successor in summer but happily switched to developing our new application xorriso as soon as it was a valid excuse.

Beautification of libburn is a slave work and not without dangers. It is not the minefield any more which i found in december 2005 but still any change can have undesired consequences. So you have to expect frustrating "what benefit" questions regularly. Sorry in advance.

It would make more sense if one would revive development of libcevap, the successor of libburn. To have a model and to be sufficiently neat is one of its main design goals.

So if you want to do rewarding work, with no frustrating legacy and sentinel, then have a look into directory ./libcevap in the SVN of libburn.

Especially: libcevap/libdax_model.txt which is a compilation of attributes in the obscure notation of my little class generator. One may well decide to take this only as checklist for challenging an own design.

For details of CgeN notation see http://libburnia-project.org/browser/libburn/trunk/libcevap/cgen.txt

Then there are GIFs made from ArgoUML model drawings (one can draw with Argo, but doing anything else needs a long carreer as MS-Windows kid).

My knowledge about MMC operations of the various media types is recorded in http://libburnia-project.org/browser/libburn/trunk/doc/cookbook.txt

Well, these would be the parts of which libcevap would be constructed. So if you want to do sincere, demanding and own work ... :))

Another topic where libburn could need improvement is media: DVD DL and BD-R/RE. Needed would be hardware and a skilled tester who does own experiments in libburn's code until it works.

  Changed 1 year ago by pygi

Thomas, to apply a patch:

patch < typedef.diff

  Changed 1 year ago by vreixo

On the risk to appear stubborn: where is the benefit in the types ?

I think they help to have clear code... I like them.

in reply to: ↑ 2   Changed 1 year ago by pfritz

Replying to scdbackup:

On the risk to appear stubborn: where is the benefit in the types ? Any change has a benefit / risk ratio. The risk may be very small. But with 0 benefit it still has 0 ratio.

Of course there is no benefit in the sense of a new feature, but I think to have a good and coherent API is a benefit as well. Since I've saw that there a typedefs for the structures in libisofs as well, I thought it is good thing to add theme in libburn, too. (I personally prefer typedefs for structures)

In general it makes it easier to learn a new API if the sub projects share the same coding style, i.e. have the same naming convention etc.

------------- General considerations before you start to work in libburn:

Just to warn you, although I'm following interested, I don't know if I have the ability and the time to help there much.

You need to teach me how to process your patches. Needed is a trustworthy shell procedure, step by step for elderly people.

Like pygi said.

It would make more sense if one would revive development of libcevap, the successor of libburn. To have a model and to be sufficiently neat is one of its main design goals.

Interesting I'll take a look into it.

follow-up: ↓ 8   Changed 1 year ago by scdbackup

Looks like i am facing a coalition of typedeffers. :))

You guys are aware that this not only affects libburn.h but also the .c files and all our libburn apps: cdrskin, telltoc, libburner, libisoburn, xorriso ?

(It would be even more ugly to define types and further on continue using the structs in the code.)

So this whole operation is not without workload and intrinsic risks of large scale code editing.


pygi: patch < typedef.diff

I have feared that answer.

My question should rather have been: "Is there a way to apply patches that does not give me creeps because i allow a program to alter my whole disk content according to lines written in a patch file ?"

Above command is nearly as safe as double-clicking a link in MS-Outlook. (Call me paranoid. :))


Summary:

What is in the patch so far, could as well be done in the application: nice alias types for our structs. According to all my experience with C compilers this should work without changes in the code of libburn.

A complete change from struct to type would affect the whole library and all our own apps. External apps could stay with the structs as long as they want, but our own ones should better follow such a move.

So if i hear now a chorus of three voices singing: "We want it so" then i give in and put it on my todo.

But i have remorse doing it as patch. I would feel much better with real handwork.

Nevertheless, consider what we will win (few) and what we risk if we touch any function of libburn and lots of our app code (well hidden bugs).

All in all i got a bad feeling with that plan.

follow-up: ↓ 9   Changed 1 year ago by scdbackup

The more i think of it, the more it appears as a subtle break of the API:

If there is no difference between type and struct, why not simply define the types in libburn.h but leave the function prototypes and all other code unchanged ?

If there is difference, which may be nearly always of no concern, then we break the API if we change the prototypes and/or the code.

in reply to: ↑ 6   Changed 1 year ago by pfritz

Replying to scdbackup:

Looks like i am facing a coalition of typedeffers. :)) You guys are aware that this not only affects libburn.h but also the .c files and all our libburn apps: cdrskin, telltoc, libburner, libisoburn, xorriso ? (It would be even more ugly to define types and further on continue using the structs in the code.)

Yes, I am. But the most work can be done with sed. And the patches can be applied step by step, to be sure that there won't be any conflicts between the svn version and the development versions on your harddisks.

pygi: patch < typedef.diff I have feared that answer. My question should rather have been: "Is there a way to apply patches that does not give me creeps because i allow a program to alter my whole disk content according to lines written in a patch file ?"

How do you normally accept work from people that don't have commit access? I really don't get your point here. It is standard (I don't know another way) in the OSS world to send and use patches. Of course it is up to you to review them.

What is in the patch so far, could as well be done in the application: nice alias types for our structs. According to all my experience with C compilers this should work without changes in the code of libburn.

That's true and maybe some will do that or are already doing that.

A complete change from struct to type would affect the whole library and all our own apps. External apps could stay with the structs as long as they want, but our own ones should better follow such a move. So if i hear now a chorus of three voices singing: "We want it so" then i give in and put it on my todo. But i have remorse doing it as patch. I would feel much better with real handwork.

What's the difference? If I write a patch and send it to you. It doesn't mean that it wasn't real handwork for me, although it is just one line to apply it for you.

Nevertheless, consider what we will win (few) and what we risk if we touch any function of libburn and lots of our app code (well hidden bugs). All in all i got a bad feeling with that plan.

Then let it as it is. I prefer typedefs, but I can live without them.

in reply to: ↑ 7   Changed 1 year ago by pfritz

Replying to scdbackup:

The more i think of it, the more it appears as a subtle break of the API: If there is no difference between type and struct, why not simply define the types in libburn.h but leave the function prototypes and all other code unchanged ? If there is difference, which may be nearly always of no concern, then we break the API if we change the prototypes and/or the code.

There isn't any API break. The only thing that will change is the code appearance. If it makes no difference for you and still fear that there is somewhere the devil inside, you better keep the code as is.

  Changed 1 year ago by scdbackup

It is not so much the devil but the inner consistency of the change. How do i justify it to myself as app programmer ?

The argument which is used to appease my remorse implies that it does not matter at all whether one uses as prototype

int burn_drive_get_adr(struct burn_drive_info *drive_info, char adr[]);

or

int burn_drive_get_adr(BurnDriveInfo *drive_info, char adr[]); 

So why then not keep the old function prototypes and only introduce the types as aliases ? The application can then choose whether to use structs or types in its code.

On the other hand struct burn_drive_info is not really the same as BurnDriveInfo?. In that aspect we would introduce a hack interface between the typed world and the callers which use the old struct definitions. So my proposal is to implement the hack already in libburn.h.


How do you normally accept work from people that don't have commit access? I really don't get your point here. It is standard (I don't know another way) in the OSS world to send and use patches. Of course it is up to you to review them.

Sorry if i gave you the impression that i am not open to OSS traditions.

The problem with a patch is the review indeed. While doing a thorough review i can as well do the changes in the code by hand.

What would rather be needed is enough coordination so i can delegate the responsibility to the contributor. The way how the contribution then is merged into the software is only the final step.

Coordination in this special case is about the sense and consequences of the proposed change. What do we want to achieve with it and where could it bite us ? (After all it's the API we're fiddling with.)

  Changed 1 year ago by pygi

Thomas, would setting up ReviewBoard? help you in reviewing? :)

http://www.review-board.org/

  Changed 1 year ago by scdbackup

Thomas, would setting up ReviewBoard?? help you in reviewing? :)

Will it carry my both hats ? The one of the library developer who would give in to the quite harmless wish of a contributor and potential user - or the one of the app developer who is opposed to anything that could endanger the foundation of cdrskin or xorriso ?

I have to find an opinion towards pfritz' proposal. Best without giving him the impression his reasons would not count or his contribution was not welcome.

But as it is now, i am the one who is responsible for accepting or rejecting - just two weeks after we finally found a viable way to ensure binary compatibility at runtime.

So the contribution is valuable because we have to consider it in the light of outmost API and ABI stability. We are still practicing.

Note: See TracTickets for help on using tickets.