api(Windows): Remove the ordinals attached to exported functions #651
Conversation
|
One more question to @jbowler, about the authorship of the awk scripts: I looked in the git log, but it's not 100% clear to me, which ones of those scripts were written by Glenn, and which ones were written by you then copyrighted by Glenn. Could you please clarify your status as an original author or a contributing author? |
I don't think Glenn ever wrote an awk script. The old sed runes, maybe. but not awk. He always claimed copyright over the whole work (which is correct since it is a composite work created by the maintainers in sequence) and therefore, as I understand it, he added his copyright to the whole. This is why I started putting stuff in "contrib", because the edits Glenn made to those contributions are, in fact, editorial; copy edits. Copy edits are just copy edits, they don't grant copyright. Have you ever seen a newspaper copy editor get a mention, even though the work they do is invaluable? I'm the original author and copyright holder of this script from the
When I submitted scripts to Glenn I deliberately did not claim copyright. I won't ever make that mistake again and, believe me, the copyright offered will by default be GPL (much as I hate GPL) unless I determine it is functionally useful to the rest of the world to use another licence, such as libpng v2 (cf my recent PRs). The other "scripts/*.awk" files, well, I'm pretty sure they are my original works and that any decent AI authorship detector would prove so; my code is just so distinctive :-) But quite frankly Glenn's estate can have them and feel free to go back to the previous versions, the sed strips. Please don't wave your hands above your head after you do this :-) Begone thou foul bolts of |
scripts/checksym.awk
Outdated
| @@ -62,67 +41,36 @@ FILENAME == master { | |||
| next | |||
| } | |||
|
|
|||
| # Read new definitions, these are free form but the lines must | |||
| # Read the definitions, which are free-form, but the lines must | |||
There was a problem hiding this comment.
Unnecessary: suggesting spelling or grammatical corrections on prior authors' comments reduces readability of a commit. I.e. "live with it".
There was a problem hiding this comment.
If this change it works it works, if it does not it does not; who cares about any change to this script?
The program only seeks to ensure that ordinals are not repeated and are consecutive up to the recorded last item. It is a "release" validation so it can be completely removed from the build process and just done as a release validation.
The maintainer should just remove this chore from the developers and DIY; do the ordinals as a last step!
| #ifndef PNG_EXPORTA | ||
| # define PNG_EXPORTA(ordinal, type, name, args, attributes) \ | ||
| # define PNG_EXPORTA(type, name, args, attributes) \ |
There was a problem hiding this comment.
Nah: don't remove the ordinal just ignore it.
The stated aim is to make the code of the public header files more readable but this change does absolutely nothing to accomplish that aim; removing a now meaningless parameter does not make the whole mess understandable in any way.
Simply ignoring the ordinal in the way you describe does, indeed, prove that it is no longer necessary (once the world uses that version of libpng), then you can see if changing the function-like macro breaks anything. The latter is far more likely. C23 varags macros might help, or not.
scripts/prefix.c
Outdated
| @@ -1,13 +1,14 @@ | |||
| /* prefix.c - generate an unprefixed symbol list | |||
| * | |||
| * Copyright (c) 2025 Cosmin Truta | |||
There was a problem hiding this comment.
The problem that I have with this approach is that this change to scripts/pngprefix.c is a copy edit.
Copy edit: correction of the original author's use of the language. No change to the original work; pure political correctness. No copyright.
Copyright: a meaningful change to the functionality of a piece of code which creates a composite work of the original author and the changer(fr).
Problem: no original copyright available, yet international law ascribes the right to the "original author". That does not, so far as I can see, require the author of a meaningful change to go forth and search.
Answer: this is not a meaningful change. The one to "scripts/checksym.awk" certainly is.
scripts/sym.c
Outdated
| @@ -1,13 +1,14 @@ | |||
| /* sym.c - define format of libpng.sym | |||
| * | |||
| * Copyright (c) 2025 Cosmin Truta | |||
There was a problem hiding this comment.
Seriously: you can't claim copyright on a one line change that was necessitated by a change you made elsewhere:
You changed the language (the correct usage of PNG_EXPORT***)
You fixed the code to conform to your new language
You have no right to claim copyright on the basis of that change.
You might want to consult a copyright lawyer in the country of your choice: I see that you are an editor of a composite work, "libpng" and can reasonably claim copyright over the whole resultant work. That's what I assumed Glenn's copyright claims meant and, I have to admit, I didn't much care.
In order to make ABI compatibility guarantees about the Windows DLL, we had to keep maintaining the API ordinals consistently. As we are now designing a fresh new ABI, we use this opportunity to stop using ordinals altogether. Exporting API functions by ordinal numbers instead of names was a necessity in the early (16-bit) versions of Windows, by design, to minimize the memory requirements. Although using ordinals was still possible in all subsequent versions of Windows, it was no longer necessary from the 32-bit versions of Windows onwards. The libpng DLL will still have ordinals, assigned arbitrarily, but the dynamic linking of applications to this DLL will be done by name. (The applications that use the zlib DLL operate in the same manner.) Reviewed-by: John Bowler <jbowler@acm.org> Signed-off-by: Cosmin Truta <ctruta@gmail.com>
Kthx, that answers my question. I credited you below the copyright notice, as I do with any other non-trivial piece of code that's written from scratch by a contributor. And of course, you are listed as a contributing author for libpng in the AUTHORS file. |
|
START OF OFF-TOPIC COMMENT
I agree with your point in principle, but I disagree with certain details based on my past interactions with legal counsel. Speaking about the principle, I don't credit trivial editorial changes like comment fixes or typo fixes in comments or code (e.g. fixing a misspelled internal variable) in a PR. And now, off we go with the details. Please keep in mind that I'm just formulating my non-expert understanding, which may or may not have been influenced by actual lawyers whose competence I shall neither endorse nor detract from. The number of lines of code is irrelevant. For one extreme example, I could write a PNG codec from scratch in TypeScript, transpile it to JavaScript, and minify it into a single line of code. (I think you'd agree about this thing being 100% copyrightable.) Conversely, I could write a rather unique hello-world program, akin to Randal Schwartz’s imaginative "Just Another Perl Hacker" signature. I could copyright this program and engage in copyright shenanigans, which would make me foolish of course, but without actually invalidating my hypothetical copyright claims. What we debate here is also irrelevant. What others argue and a judge decides, well, that is relevant. My understanding (please don't quote me!) is that the judge's decision will be based inclusively (not exclusively!) on the obviousness of the solution to the problem, not on how "editorial" the modifications are. If there's an obvious solution, then the copyright to that solution is likely not enforceable. Note that enforceability and copyrightability are different concepts. Back to our concrete situation: I 100% agree with you John, without any argument whatsoever, that I could not possibly enforce a copyright on those copy-editing modifications that you outlined, even though they do result in changes to a machine-generated output -- specifically, the C preprocessor's output. (I mean... who you' kidding, that thing wouldn't even compile otherwise!) What I do not agree is the place where to draw the line. If you have a better idea, I'm all ears. (Seriously!) But it has to be a hard and fast rule, not only for my verification and maintenance scripts ("oops I forgot to update the copyright year") but also for myself: to draw a firm line in a manner in which I can ensure that the copyrightable code is copyrighted. And, finally, the conclusion: As a personal rule of thumb: I'm applying my own copyright, and I'm giving authorship credits in the AUTHORS file if applicable, to all modifications that result in changes in the output of a language processor (or preprocessor or postprocessor) that are actual solutions to actual problems. This is regardless of how big or small (or even trivial) the solved problem is, and regardless of how complicated or simple (or even trivial) the solution is. Having said that: I am also willing to change this rule of mine, if someone can convince me otherwise. END OF OFF-TOPIC COMMENT |
|
Ok now, back to our business:
I am no clairvoyant, but I know what happens: nothing. Which is the same as what happened to Zlib's DLL after I made the same "radical" change about 20 years ago. (https://www.zlib.net/DLL_FAQ.txt)
I actually do have an answer to this comment also, but let us leave that other off-topic story for another code review session.
Hear! Hear! |
ctruta
force-pushed
the
libpng18
branch
4 times, most recently
from
11c376f to
c4780f3
Compare
|
So I acquiesced to your observations about not wanting the commit to be too big, and, before the final push, I rolled back a few changes in comments that I had originally made here and there. ... for now 😝 Look, John, there's a lot (a lot!) of cruft in libpng that slowly and steadily accreted over the last quarter-century. We said that we don't break backwards compatibility with the existing applications, because we don't. However, the libpng codebase isn't getting any younger, unless we do the necessary work to rejuvenate it. Which we can, and we should. png.h still doesn't look so "young" in commit c4780f3 but in my imagination it already does. Thank you for this review. The next big honking refactoring commit is coming up. |
@jbowler please review.
This is only the beginning of the function ordinal cleanup. Whatever I made out of scripts/checksym.awk is kinda gooey ("lame, but working"), and it should be cleaned up much more, if not actually removed altogether.
It's png.h that preoccupies me, though. We have the chance to restore readability. We should make the functions look like functions, to the best of our abilities, without breaking the existing build workflow.