Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change port array in PROGRAMMER to be const char * #1699

Merged
merged 6 commits into from
Feb 25, 2024

Conversation

stefanrueger
Copy link
Collaborator

@stefanrueger stefanrueger commented Feb 22, 2024

The port array in the PROGRAMMER structure is wasteful and can cause a buffer overflow by the way the command line -P string used to be just copied into the array. Here what happens if you have a 5000 character random -P string:

$ avrdude -P $(tr -dc 'a-zA-Z0-9' </dev/urandom | head -c 5000) -c urclock
*** buffer overflow detected ***: terminated
Aborted (core dumped)

With this PR, the programmer structure is much smaller and no buffer overflow happens here:

$ avrdude -P $(tr -dc 'a-zA-Z0-9' </dev/urandom | head -c 5000) -c urclock
avrdude OS error: cannot open port gGZVqWzKXPLZ63O95NwX2KcaWpsO1elgoVj0w9KnrNuTNYMOs87b5sKp6lnO5EjIVdzB9cB5yyXZw6ksrTWiDBZh0drVqaDfbZk0geE5GrzpSav4oXdJojGysEjkC0Ql8VIkQG4pl4NOK7ZryT5oknqegidz8ZCfDVAWQxw3XiKbNfQLJGD2ZLHLoOIjPYz9dLxYSLN6yGI3uQb9MCXv8orJOU5GYaHD33ZijmSsb9YoM7GriYP1QuGxpEDu3ddWLEzefr5l090YTH9wtpaGdrqSboQeQ7CLPZzSQ3qu5cYvSc9iDNgRwKlxTRdCwmFL9B58RAunaGtHr97Dwqs3EHL0mqWvnuFpvv64jVGwOwteGNM13KTVose5xO7zWAk3upPsfocNgFziJCfmPJyFEUUYFYjIrcLvvY6hIGoOzxIQQNGpFOD9hxCQzTHBXENJ6go2uoNUbrTWLw7AnzWw93iXeOOIcxq0u98w2lwSPXqJQvv6Q98BTekOcbLYtVmqeUFM5qohAbHnlzu7xesgTmFCGqKggzaawCFBzseMzf9vMcB55QW1YxaYbvqRFQcGTsUVjCUeEFTXe0O6z0A9PZrHX0a4hmNYIWaaOfc9Pwy94IVT1tc2sAXUPQuYgeN4kvLZLCvgQy8RwOMGreUzICNLzhJsPTwxmi8hQQ7p98tU29CaMdReyXl6BFZHbbmdAsN9xKPkmC9Bzy0b36vMKpEGGBHmP1gtpAVTwuiv0EhMq62Mfdhb1LEt5PEETh2NIANxvT5pDEgwAr8Y57ODRRSJ2twuYnbOCNtSUKEc5klgJ4hMK2qHmBNlHWBlIfJTyDLonMRKM8lVNYOmVaUwZOZIxU3Z6dheh7t2UtH6XfPpQ0Z518BxjekVP3wrmlFfUvcPdDgbmQB79SXcUffolWLJFspvfJcdBjCb1MMfZufdPbFAW6oygd3HIL4fkkbOAlbxVgdK8ydcgBNs9MwC4p5qHSLEsF6VZKSLj7boTZSUgqhOAXwDaBzUd0D53d6sum364EMQV0RNEYwTt0qgPCe4aEt6NuU92Eb3ObZnkQtibYnGdEzuNpjBdZMeHsjDWta4J1zHXMFnzK0tFySIF8c1OIC5IE0iL4LFRMP56hGSy82VkTplq4RhpuyFqC7MH1Yfof3T0ArauXgy8pwK2S4TolJq6iajHOt3APIrau8XD3zIAKvArZTQiX6EI4m9JNzpxO9J292CCz31CMiUaHWUYPtyDo95MVH1rxIMpwQ9zEktCPb949v5i8fYvaRHHdtdKS99HHAaBnrDuM5XMGloNyBCw1CzenH78VkctTDDrTK5ynuaA7tFtTdakTWPhkwj4thqBZtsR0E1FcT8efLjMVGDGI0gecQD4M556HbY0eEqeFW0f2Lx8NUd7vlopUd8lozpElmBkWbsr0GvSzjStXIfFJz0lbhKIQk39cMxgSZEiOZjFp2xoTXtB1GRh3K2z5Evjfv79HwfjhwpnM4o1hEP2X6DGdJEpD3Dgry5waZQlGhxLxWSONsDpcuJdYX402kRsWdaU1AXRsJLHjp9Ytq16T43nILBlsVAyHFH7GpPQ8GPpxGiv1I5e6nlztI7xhiBDF2vpoJcfDH6RPyUvLXJ1bF9MvoxxfbQOav6Pdona2zfIla2I0f0xAcibzMJg026OWzRcYLKKarpS5OHzkjAO3PMOJWe0gglwqcTTeeQDx80tqqiA75lV3ADb1DHELTK1RuIEDOHmAAV6TPMtuCYKpeT14jU9n85bcUFaNEGyf9uFnmGTcL39ZD5Uwy7nJmHB8dEfnYAvTvYR61d7cfq9Z83MfxfdcWKjKkDfDDhRFifIbJDNBDE9Vwkkeeqe1nY4211aoioAfKUytcsJb1oPmeyHCX8gIdNLevARvYgJpb8m7ECHMD13hd46oltPTwLE5HsyGc1d7HCbAhl1kOGGq1RMsJRTZyMK6PziezrGoafhUiu77pEurLve6JprnMAPVCHnBI6ceI6xZZmB5mpbfVJXhRrjMSN5e8s2Js91MKsRgpQyyVDLkR8ANwAJTOD149cfSQAtP6q9MrTSILGMeAGjryoSd439kecuQ58aZy840rEmjd3s8Y5ZcIUJuBMR0IaVwGdWuBFcSmwrtb6Sm1mQh4XxiMx4ma9meoerro1BMdQhXIV0g700Oh3kVRFc2zU93USBiSqR1wB12tDjtFoMtM278XoCXRfXUdqSbc3UJKf9zFf8dxaUOpbfSU0blKPD8N5piO4qhpJfijkXJsG29Rv4FHeggAzHsXjkBKglgQPQEeX1yuGwXEyMkDZ3fKns4rijLvpmgQyEcL9RMZg8kgkEIW7m5sUYyStpseqtzZkuGpH8D2OFlcybNAJ7xzXFN4ASTVr2XZXN05KqV0v3ob4TsbHyRsFgvg6HhBr5h9DFendLuOLb21Ae8tY6hZSjKt11rTMsm6nNOjTsAXvQsM7dHbaHaLruFXFNBckuKmbZEVbQRidpowfWiYItbkyUx45dgK5yB9LGwytyA2QRqXqnHXMHFEnJTYLkLne8oy9UIIFGMAD5GIZrFHpos1x1cCRZaXi0X6VfRWKNd7IwmgmZxuhvukmbu01dgDTnY0k3AHQ1AlIhr3Gkr1DHbbnS7obCTJ6rg3PMbQfU6VX7oPqyA7EywZC9g8e5UzRIRgDwIExaXs7vzM1Ckcks8IDcpbumjMjNs1Yo73lviuaF4fciNh8C0hJLSYGXuaYpOMGVKxevt2t9LlXMI9lex4FqLRqTRjc3iMyI8Fj1TwxLhMC9inKzc0IY15C4QUF9snx1Tygtxhl3yd3MwqnkYoDM2MS10nEibfWKZogAiwtPx1e3WDzXAjgwTPJq1WxNc6CGhcBRl8x5BzD1xyOWFi4LUHjfawGccYNQmu4mKnmv2QtRSkve5rf5AzCoepCraSiZxEI93ptSqA3i4BvuYcTPpi4iWbNKFh03vsGXLNJ68vVnQLOfwdE4uV0eU158BVGWdSf7dzQv6Mbv88Pvll2Xpa8FEZgSI8an07h4AwuwYl81EN96QN3Da2fPL8TMNTmxGcrM0L3TtBHOKExMlKj1G99SHYH6Ba85Yas0t332xyOBTfqvg8uoHI1HDKefWmmc91gzq9QSZ80Sk0YMsC78dX2ETWCG29EZYKEnASCMOdh8LE7oC6pOFKprQQMTUSXc9knZEdoJvSpHDQUpvMzD9YKCxbcUVMCbfNrcD0ejWGQxJM5bqE6YaDmgOfCfRLvbwPeBM1I8FmWofBvK9aYSuHvhtlGpfrwXuPn87rqSkiEQsBPvypDUrJTrmIyKwGxtBPKLX4WAA46CTtzxvmRxUN4rMDUUjG1miPD47SBshRKLC3xVHmvWNj1ryX98b5fZH5aDDxGiJxzhSz0wXyRhjBJvs5BwnR0BLfOD5TICqND6eU6rbyfyEKHV9n61wzg8VQ2JSiCSVZJarDKhlzj13AuPxMBvOa1lZOkaZN3B3L4NLSr7IoKm5RhMECDszNHxEpzXYp6tWAkHe9MeMfKvu9vJa1E7E3ZPtKstYEl9JdPYsWriRWbzFTRh5w7pxBkYGgWpv2WXeS3ebCmuVbq088XH2GpSiii5uZCQ4AYDy19yNsNnQJTLhuRzhauT9Xc8BYDy9m19LIoVxx2TVdMGexQXu2PVJzmDEIkL2mHBWi55eGOiA3uxHTOiVsbqkub6YXjj3fjNAk7FJSHdvSVczc44d1EavvWlBhepQzSysQjb6xiAAgvbED0SIEmSMUErhV5BJmCwyK9qHx3zTmQgIV3GIm7BmvfsZoMTl2MEywIkMMPKHXMUkAYSptZjm0tEN0meeLwvTdvfbBiN49nNVDPT0HIuMkQKNQJlvHMS14nMH5g37XEpWlZDOXgIM3cOODyOTBpnWfmB1K4ULSSIM6sCOG5M8gBe9vdKo8uq3iVR3zUxvYmubtz2XV5y3EEujwiWrWhpaqi23yrJHgFgDPwvVF88zbV1psP5LtrnsGiMrRQNKZRBeyS1Be01f1MRnyOV9h2jCMDKW7ZMjbi3lZlEAnxB5cu9fXNU6MWeQRo7VNvbV1RaqIJrWmTw3STR8kiyBpVewBnugHSARI2mYFhi2U5SLzJ4nkzMzuFlDXkG8TqPu1DHPhXbrj557nrqPKZVIv9o0S5Ql89QHVd72NpJPMxqB2On3vfQ7rnIArvHWQx4j442y8cRlZhGruy7ZKSJH3b6UujxPsk7MkQWghu5gmMmGNfSgKnYBUmFpgkq1wDwC5ypFnmGFRRf2Aa6rMPjszXVgt1PEWbe0bzZu3QXgT9zVu0ywQD1oMouTrEY3aQU9jQwZGYEiHgVxbyWcLV22UK5rMXu8Eugc8X6I2LCxxbrNEkMB67MWvUUiSuuleK0dl4jyWDNOHqiacltw6XmkR32CXYniwjrXaAN8vXfwDP4lBfK6HZchTfkxBRxphIT35yAj5RvK8DsGyMHpiwbsEZGuQSc1APHumatBm2rnobTlfUfQaeTMgCTiS9ei35iNwPXZSSpKdrppxNd5KvADJwRYjQ9iFExJt2xlU9RD8nQJGBPsufaOjDfJtsixYzDGtKOabh9XPdk3yBlKgPxq1tPEuR7V5HlmTEemlWsrL99UUMFpnwiBlEVV5Y3tbTj14i5xaY4tCANM1nvNV5cuexcAJhbBm02Z8Dm3JfRJGxKuDZ8cMSq7BupHJ2uGw22uI36jCZIYicrOjcOCXR47F6FRR2xvrqRO3rmgHeZYYsnhAoz7tJ1ZBlUebKizZ96b9x3fhRloqIdCHHYyte1UHNANR0SCwwG0JYTk3nehyts4Ob6bQP5WslCOx4DUGhx0YbXYWMbntB9I7nrXKeEzQHgnfztg0992tJZwqAPvyP88qMEFaqfb1NUG9fyTH9b2Nl9pnnGlUeRXCcrv0bIUCykCFv3Xyt0eC03q2KzLHXHQOy6PIvEfgWcV0SIcpYTI47gkblzi8GKAqjE2LbIkmh9hiVbBEk: File name too long
avrdude error: unable to open port gGZVqWzKXPLZ63O95NwX2KcaWpsO1elgoVj0w9KnrNuTNYMOs87b5sKp6lnO5EjIVdzB9cB5yyXZw6ksrTWiDBZh0drVqaDfbZk0geE5GrzpSav4oXdJojGysEjkC0Ql8VIkQG4pl4NOK7ZryT5oknqegidz8ZCfDVAWQxw3XiKbNfQLJGD2ZLHLoOIjPYz9dLxYSLN6yGI3uQb9MCXv8orJOU5GYaHD33ZijmSsb9YoM7GriYP1QuGxpEDu3ddWLEzefr5l090YTH9wtpaGdrqSboQeQ7CLPZzSQ3qu5cYvSc9iDNgRwKlxTRdCwmFL9B58RAunaGtHr97Dwqs3EHL0mqWvnuFpvv64jVGwOwteGNM13KTVose5xO7zWAk3upPsfocNgFziJCfmPJyFEUUYFYjIrcLvvY6hIGoOzxIQQNGpFOD9hxCQzTHBXENJ6go2uoNUbrTWLw7AnzWw93iXeOOIcxq0u98w2lwSPXqJQvv6Q98BTekOcbLYtVmqeUFM5qohAbHnlzu7xesgTmFCGqKggzaawCFBzseMzf9vMcB55QW1YxaYbvqRFQcGTsUVjCUeEFTXe0O6z0A9PZrHX0a4hmNYIWaaOfc9Pwy94IVT1tc2sAXUPQuYgeN4kvLZLCvgQy8RwOMGreUzICNLzhJsPTwxmi8hQQ7p98tU29CaMdReyXl6BFZHbbmdAsN9xKPkmC9Bzy0b36vMKpEGGBHmP1gtpAVTwuiv0EhMq62Mfdhb1LEt5PEETh2NIANxvT5pDEgwAr8Y57ODRRSJ2twuYnbOCNtSUKEc5klgJ4hMK2qHmBNlHWBlIfJTyDLonMRKM8lVNYOmVaUwZOZIxU3Z6dheh7t2UtH6XfPpQ0Z518BxjekVP3wrmlFfUvcPdDgbmQB79SXcUffolWLJFspvfJcdBjCb1MMfZufdPbFAW6oygd3HIL4fkkbOAlbxVgdK8ydcgBNs9MwC4p5qHSLEsF6VZKSLj7boTZSUgqhOAXwDaBzUd0D53d6sum364EMQV0RNEYwTt0qgPCe4aEt6NuU92Eb3ObZnkQtibYnGdEzuNpjBdZMeHsjDWta4J1zHXMFnzK0tFySIF8c1OIC5IE0iL4LFRMP56hGSy82VkTplq4RhpuyFqC7MH1Yfof3T0ArauXgy8pwK2S4TolJq6iajHOt3APIrau8XD3zIAKvArZTQiX6EI4m9JNzpxO9J292CCz31CMiUaHWUYPtyDo95MVH1rxIMpwQ9zEktCPb949v5i8fYvaRHHdtdKS99HHAaBnrDuM5XMGloNyBCw1CzenH78VkctTDDrTK5ynuaA7tFtTdakTWPhkwj4thqBZtsR0E1FcT8efLjMVGDGI0gecQD4M556HbY0eEqeFW0f2Lx8NUd7vlopUd8lozpElmBkWbsr0GvSzjStXIfFJz0lbhKIQk39cMxgSZEiOZjFp2xoTXtB1GRh3K2z5Evjfv79HwfjhwpnM4o1hEP2X6DGdJEpD3Dgry5waZQlGhxLxWSONsDpcuJdYX402kRsWdaU1AXRsJLHjp9Ytq16T43nILBlsVAyHFH7GpPQ8GPpxGiv1I5e6nlztI7xhiBDF2vpoJcfDH6RPyUvLXJ1bF9MvoxxfbQOav6Pdona2zfIla2I0f0xAcibzMJg026OWzRcYLKKarpS5OHzkjAO3PMOJWe0gglwqcTTeeQDx80tqqiA75lV3ADb1DHELTK1RuIEDOHmAAV6TPMtuCYKpeT14jU9n85bcUFaNEGyf9uFnmGTcL39ZD5Uwy7nJmHB8dEfnYAvTvYR61d7cfq9Z83MfxfdcWKjKkDfDDhRFifIbJDNBDE9Vwkkeeqe1nY4211aoioAfKUytcsJb1oPmeyHCX8gIdNLevARvYgJpb8m7ECHMD13hd46oltPTwLE5HsyGc1d7HCbAhl1kOGGq1RMsJRTZyMK6PziezrGoafhUiu77pEurLve6JprnMAPVCHnBI6ceI6xZZmB5mpbfVJXhRrjMSN5e8s2Js91MKsRgpQyyVDLkR8ANwAJTOD149cfSQAtP6q9MrTSILGMeAGjryoSd439kecuQ58aZy840rEmjd3s8Y5ZcIUJuBMR0IaVwGdWuBFcSmwrtb6Sm1mQh4XxiMx4ma9meoerro1BMdQhXIV0g700Oh3kVRFc2zU93USBiSqR1wB12tDjtFoMtM278XoCXRfXUdqSbc3UJKf9zFf8dxaUOpbfSU0blKPD8N5piO4qhpJfijkXJsG29Rv4FHeggAzHsXjkBKglgQPQEeX1yuGwXEyMkDZ3fKns4rijLvpmgQyEcL9RMZg8kgkEIW7m5sUYyStpseqtzZkuGpH8D2OFlcybNAJ7xzXFN4ASTVr2XZXN05KqV0v3ob4TsbHyRsFgvg6HhBr5h9DFendLuOLb21Ae8tY6hZSjKt11rTMsm6nNOjTsAXvQsM7dHbaHaLruFXFNBckuKmbZEVbQRidpowfWiYItbkyUx45dgK5yB9LGwytyA2QRqXqnHXMHFEnJTYLkLne8oy9UIIFGMAD5GIZrFHpos1x1cCRZaXi0X6VfRWKNd7IwmgmZxuhvukmbu01dgDTnY0k3AHQ1AlIhr3Gkr1DHbbnS7obCTJ6rg3PMbQfU6VX7oPqyA7EywZC9g8e5UzRIRgDwIExaXs7vzM1Ckcks8IDcpbumjMjNs1Yo73lviuaF4fciNh8C0hJLSYGXuaYpOMGVKxevt2t9LlXMI9lex4FqLRqTRjc3iMyI8Fj1TwxLhMC9inKzc0IY15C4QUF9snx1Tygtxhl3yd3MwqnkYoDM2MS10nEibfWKZogAiwtPx1e3WDzXAjgwTPJq1WxNc6CGhcBRl8x5BzD1xyOWFi4LUHjfawGccYNQmu4mKnmv2QtRSkve5rf5AzCoepCraSiZxEI93ptSqA3i4BvuYcTPpi4iWbNKFh03vsGXLNJ68vVnQLOfwdE4uV0eU158BVGWdSf7dzQv6Mbv88Pvll2Xpa8FEZgSI8an07h4AwuwYl81EN96QN3Da2fPL8TMNTmxGcrM0L3TtBHOKExMlKj1G99SHYH6Ba85Yas0t332xyOBTfqvg8uoHI1HDKefWmmc91gzq9QSZ80Sk0YMsC78dX2ETWCG29EZYKEnASCMOdh8LE7oC6pOFKprQQMTUSXc9knZEdoJvSpHDQUpvMzD9YKCxbcUVMCbfNrcD0ejWGQxJM5bqE6YaDmgOfCfRLvbwPeBM1I8FmWofBvK9aYSuHvhtlGpfrwXuPn87rqSkiEQsBPvypDUrJTrmIyKwGxtBPKLX4WAA46CTtzxvmRxUN4rMDUUjG1miPD47SBshRKLC3xVHmvWNj1ryX98b5fZH5aDDxGiJxzhSz0wXyRhjBJvs5BwnR0BLfOD5TICqND6eU6rbyfyEKHV9n61wzg8VQ2JSiCSVZJarDKhlzj13AuPxMBvOa1lZOkaZN3B3L4NLSr7IoKm5RhMECDszNHxEpzXYp6tWAkHe9MeMfKvu9vJa1E7E3ZPtKstYEl9JdPYsWriRWbzFTRh5w7pxBkYGgWpv2WXeS3ebCmuVbq088XH2GpSiii5uZCQ4AYDy19yNsNnQJTLhuRzhauT9Xc8BYDy9m19LIoVxx2TVdMGexQXu2PVJzmDEIkL2mHBWi55eGOiA3uxHTOiVsbqkub6YXjj3fjNAk7FJSHdvSVczc44d1EavvWlBhepQzSysQjb6xiAAgvbED0SIEmSMUErhV5BJmCwyK9qHx3zTmQgIV3GIm7BmvfsZoMTl2MEywIkMMPKHXMUkAYSptZjm0tEN0meeLwvTdvfbBiN49nNVDPT0HIuMkQKNQJlvHMS14nMH5g37XEpWlZDOXgIM3cOODyOTBpnWfmB1K4ULSSIM6sCOG5M8gBe9vdKo8uq3iVR3zUxvYmubtz2XV5y3EEujwiWrWhpaqi23yrJHgFgDPwvVF88zbV1psP5LtrnsGiMrRQNKZRBeyS1Be01f1MRnyOV9h2jCMDKW7ZMjbi3lZlEAnxB5cu9fXNU6MWeQRo7VNvbV1RaqIJrWmTw3STR8kiyBpVewBnugHSARI2mYFhi2U5SLzJ4nkzMzuFlDXkG8TqPu1DHPhXbrj557nrqPKZVIv9o0S5Ql89QHVd72NpJPMxqB2On3vfQ7rnIArvHWQx4j442y8cRlZhGruy7ZKSJH3b6UujxPsk7MkQWghu5gmMmGNfSgKnYBUmFpgkq1wDwC5ypFnmGFRRf2Aa6rMPjszXVgt1PEWbe0bzZu3QXgT9zVu0ywQD1oMouTrEY3aQU9jQwZGYEiHgVxbyWcLV22UK5rMXu8Eugc8X6I2LCxxbrNEkMB67MWvUUiSuuleK0dl4jyWDNOHqiacltw6XmkR32CXYniwjrXaAN8vXfwDP4lBfK6HZchTfkxBRxphIT35yAj5RvK8DsGyMHpiwbsEZGuQSc1APHumatBm2rnobTlfUfQaeTMgCTiS9ei35iNwPXZSSpKdrppxNd5KvADJwRYjQ9iFExJt2xlU9RD8nQJGBPsufaOjDfJtsixYzDGtKOabh9XPdk3yBlKgPxq1tPEuR7V5HlmTEemlWsrL99UUMFpnwiBlEVV5Y3tbTj14i5xaY4tCANM1nvNV5cuexcAJhbBm02Z8Dm3JfRJGxKuDZ8cMSq7BupHJ2uGw22uI36jCZIYicrOjcOCXR47F6FRR2xvrqRO3rmgHeZYYsnhAoz7tJ1ZBlUebKizZ96b9x3fhRloqIdCHHYyte1UHNANR0SCwwG0JYTk3nehyts4Ob6bQP5WslCOx4DUGhx0YbXYWMbntB9I7nrXKeEzQHgnfztg0992tJZwqAPvyP88qMEFaqfb1NUG9fyTH9b2Nl9pnnGlUeRXCcrv0bIUCykCFv3Xyt0eC03q2KzLHXHQOy6PIvEfgWcV0SIcpYTI47gkblzi8GKAqjE2LbIkmh9hiVbBEk for programmer urclock

avrdude done.  Thank you.

@stefanrueger stefanrueger changed the title Remove port array Change port array in PROGRAMMER to be const char * Feb 22, 2024
@mcuee mcuee added the enhancement New feature or request label Feb 22, 2024
@ndim
Copy link
Contributor

ndim commented Feb 22, 2024

This changes the libavrdude API and ABI in an incompatible way, and therefore also requires a change to the library soname.

All library users need to at least rebuild, possibly also change their source code.

@stefanrueger
Copy link
Collaborator Author

This changes the libavrdude API and ABI, and therefore requires a new soname.

All library users need to at least rebuild, possibly also change their source code.

This is correct (though in a C++ world the port component would probably be private and not be part of the API). We have neglected changing the version/soname in the past but it's good to start now! At this point I consider the changes minor, but happy to consider any different viewpoints.

@mcuee
Copy link
Collaborator

mcuee commented Feb 22, 2024

I do not see any issues to change now since no real use of libavrdude as of now.

@ndim
Copy link
Contributor

ndim commented Feb 22, 2024

The size of the PROGRAMMER struct and possibly/probably the memory layout of the other struct members has changed.

So all binary code dynamically linked against libavrdude would access the wrong memory location, and it is better for the code to fail to load at the dynamic linking stage and not run at all than the code misinterpreting the data in memory at runtime.

So the soname must change, both the SOVERSION in CMakeLists.txt and the libavrdude_la_LDFLAGS = -version-info in the Makefile.am, probably replacing the 1 with a 2.

BTW, changing the type and size of a private class member in C++ would also change the memory size and memory layout of the class, certainly breaking the ABI, and possibly breaking the API.

The way to avoid that kind of thing is to make the type opaque, both the member layout and also the total size of the type, and only give interface users accessor functions.

In C, you achieve that by using incomplete types and only giving API users pointers, never a complete type.

@ndim
Copy link
Contributor

ndim commented Feb 22, 2024

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 332bf046..50883a1b 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -279,8 +279,8 @@ add_library(libavrdude
 set_target_properties(libavrdude PROPERTIES
     PREFIX ""
     PUBLIC_HEADER "libavrdude.h"
-    VERSION 1.0.1
-    SOVERSION 1
+    VERSION 2.0.0
+    SOVERSION 2
     )
 
 target_include_directories(libavrdude
diff --git a/src/Makefile.am b/src/Makefile.am
index 8863291a..98c9b046 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -219,7 +219,7 @@ libavrdude_a_SOURCES = \
 	xbee.h \
 	xbee.c
 libavrdude_la_SOURCES = $(libavrdude_a_SOURCES)
-libavrdude_la_LDFLAGS = -version-info 1:0
+libavrdude_la_LDFLAGS = -version-info 2:0
 
 include_HEADERS = libavrdude.h
 

@stefanrueger
Copy link
Collaborator Author

@ndim Thanks for the tips! OK, if we adopt sth like semantic versoning for the library, we'd have to change the major version number. And good point about dynamic linking. We routinely change the structs as AVRDUDE gets better at modelling programmers and parts, so we are likely to see rapidly changing major version numbering (but that's the nature of trying to keep up with the world of AVR parts and Microchip's changes)

@ndim
Copy link
Contributor

ndim commented Feb 23, 2024

For shared libraries on Linux, FreeBSD, etc. the cmake library VERSION 7.8.9 gives the real name, e.g. libavrdude.so.7.8.9. The SOVERSION 4 gives the soname like libavrdude.so.4, which is a symlink pointing to real name libavrdude.so.7.8.9. And then there is the linker name libavrdude.so, which is a symlink pointing to the soname libavrdude.so.4. These numbers are confusing, so I would suggest to keep the SOVERSION the same as the first number of the VERSION, and only if we are certain there are no technical standards for the other two numbers in VERSION treat them to something like semantic versioning. Those library numbers are technical numbers first, user visible second to not at all.

Also, if the libavrdude API and ABI are that unstable, perhaps at least the libavrdude.h file as the only documentation of the API should prominently mention that so that library users have to find it.

As an aside, with the API and ABI that volatile, I will keep not shipping the libavrdude library in the Fedora avrdude package at this time.

@stefanrueger
Copy link
Collaborator Author

@ndim Sure, everyone compiling a program against libavrdude would need to utilise the corresponding libavrdude.h, and the compiled program would need to be linked to the matching library, either dynamically at runtime or statically.
Would it be possible for libavrdude.h to #define AVRDUDE_FULL_VERSION ...? Then we could declare a little string extern const char avrdude_full_version[]; in libavrdude.h and put the following version.c into libavrdude

#include "libavrdude.h"

const char avrdude_full_version[] = AVRDUDE_FULL_VERSION;

void avr_check_version(char *afv) {
  if(!str_eq(afv, avrdude_full_version)) {
    pmsg_error("compiled against %s libavrdude.h header but linked against %s libavrdude library\n", afv, avrdude_full_version);
    exit(1);
  }
}

... and comment early on in libavrdude.h

/*
  * The libavrdude library routinely changes PROGRAMMER, AVRPART and other structures to keep up with
  * changing part and programmer models. Any application that uses this library, including AVRDUDE, should
  * #include "libavrdude.h" and call check_libavrdude_version(AVRDUDE_FULL_VERSION); early on in main.c
  * to sanity check that this header file works with the linked libavrdude.
  */

@ndim
Copy link
Contributor

ndim commented Feb 23, 2024

The value of AVRDUDE_FULL_VERSION changes with every commit, regardless of whether that commit actually breaks or even changes the API/ABI at all.

@stefanrueger
Copy link
Collaborator Author

Probably best then to increment soname and library version from now on every time the ABI/API changes. Let's see how often that actually happens.

@stefanrueger stefanrueger merged commit a6b5ace into avrdudes:main Feb 25, 2024
12 checks passed
@stefanrueger stefanrueger deleted the remove-port-array branch February 25, 2024 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants