Mailing Lists: 2024

Sunday, 22 December 2024

[netsurf-users] news.novaabbs.org display issues

news.novabbs.org display issues with differing results when CSS
enabled/disabled.

Examples
https://news.novabbs.org/common/grouplist.php?groupsearch=comp.sys.acorn&Submit=Search

If you browse to above address, will show the Acorn group.
Browsing with CSS enabled, does not show the table borders and text
appears small with CSS disabled text is larger and easier to read, but
still no borders

Registration page does does display correctly with CSS enabled, but
workable when disabled

When posting the page does not display correctly with CSS i.e. no input
boxes for the fields, but these do appear with CSS disable. Also with
posting the Message box does is too narrow.

news.novabbs.org is an online client for reading and posting to
newsgroups. Whilst is useable within NetSurf, would be much easier of the
input boxes appeared correctly, without having to switch off CSS, or just
guessing where the boxes are with CSS enabled.

--
Paul Stewart Sent from A9home running RISC OS 4.42

Friday, 15 November 2024

[netsurf-dev] FOSDEM 2025 - Retrocomputing Devroom - Call for participation

Hi,

We'll have a retro devroom this year again:

https://lists.fosdem.org/pipermail/fosdem/2024q4/003591.html

François.

Friday, 8 November 2024

[netsurf-dev] Re: Toolchain build for AmigaOS4

On 08/11/2024 14:35, Chris Young wrote:
>
>
>
> On 08/11/2024 13:41, vincent sanders wrote:
>>
>> On 08/11/2024 10:48, Chris Young wrote:
>>>
>>>
>>>
>>> On 07/11/2024 10:56, Chris Young wrote:
>>>>
>>>>
>>>> On 06/11/2024 13:09, vincent sanders wrote:
>>>>>
>>>>> On 18/10/2024 15:09, Chris Young wrote:
>>>>>> Hi all
>>>>>>
>>>>>> I've had some issues this year with various things not working,
>>>>>> and tracked it down to the OS4 betas and (indirectly) the update
>>>>>> of the SDK. There were two problems;
>>>>>> 1. DNS resolution was not working. I fixed this at the start of
>>>>>> the year but after fixing (2) it broke again in a different way.
>>>>>> I've temporarily disabled the threaded resolver until I can find
>>>>>> a more permanent solution.
>>>>>> 2. SSL/TLS sites were not opening. I'm not sure why but it may
>>>>>> be the same problem as OS3 was having with OpenSSL. As AmiSSL is
>>>>>> kept up-to-date now, unlike when I started porting NetSurf, I've
>>>>>> switched the OS4 build over to using AmiSSL.
>>>>>>
>>>>>> Please can somebody check and merge my chris/amissl-os4 branch
>>>>>> into toolchains, and rebuild the ppc-amigaos toolchain?
>>>>>> The equivalent chris/amissl-os4 branch can then be merged into
>>>>>> NetSurf and the CI builds should be working again on OS4 beta.
>>>>>>
>>>>>> Thanks!
>>>>>> Chris
>>>>>>
>>>>>>
>>>>> hi chris, apologies for the delay. I applied a variant on your
>>>>> changes to the toolchain (selecting the ssl provider suppresses
>>>>> openssl so some changes you had were not required)
>>>>>
>>>>> I also updated the c-ares version which may help with the DNS
>>>>> issues (maybe)
>>>>>
>>>>> You should be ok to merge any netsurf changes you want now
>>>>>
>>>>>
>>>> Fab, thanks. I'll try with the new c-ares - it looked like the
>>>> issue was that it didn't have the code for obtaining DNS servers in
>>>> it (which I added), but it still didn't work. Maybe I did something
>>>> wrong, I'll try again.
>>>>
>>>
>>> Well, this is a bit weird. After all the updates I'm now getting
>>> "unsupported protocol", even from the Curl executable. It shows http
>>> and https in the list but neither are working :(
>>>
>>>
>> I updated the SDK with up to date versions of most libraries
>> (including libcurl) and re-built the CI toolchains. This was actually
>> because I was hoping to do a release this month amd we were behind on
>> security updates
>>
>> Maybe this will help? unsure why building libcurl with a different
>> ssl library would cause the protocol failures you see though.
>>
>
> I was wondering if the updated libcurl was the issue. It isn't the
> SSL library causing it as even non-SSL sites are returning the same
> error!
>
> I'll investigate further.
>
Turns out that there are some defines that shouldn't be there when
building against AmiSSL (or bsdsocket perhaps). I've fixed it, and also
re-tested the threaded resolver and that works again too!

If you could build the toolchain again that would be fab.

I've also checked under OS3 but that wasn't affected.

Thanks
Chris

[netsurf-dev] Re: Toolchain build for AmigaOS4

On 08/11/2024 13:41, vincent sanders wrote:
>
> On 08/11/2024 10:48, Chris Young wrote:
>>
>>
>>
>> On 07/11/2024 10:56, Chris Young wrote:
>>>
>>>
>>> On 06/11/2024 13:09, vincent sanders wrote:
>>>>
>>>> On 18/10/2024 15:09, Chris Young wrote:
>>>>> Hi all
>>>>>
>>>>> I've had some issues this year with various things not working,
>>>>> and tracked it down to the OS4 betas and (indirectly) the update
>>>>> of the SDK. There were two problems;
>>>>> 1. DNS resolution was not working. I fixed this at the start of
>>>>> the year but after fixing (2) it broke again in a different way.
>>>>> I've temporarily disabled the threaded resolver until I can find a
>>>>> more permanent solution.
>>>>> 2. SSL/TLS sites were not opening. I'm not sure why but it may be
>>>>> the same problem as OS3 was having with OpenSSL. As AmiSSL is
>>>>> kept up-to-date now, unlike when I started porting NetSurf, I've
>>>>> switched the OS4 build over to using AmiSSL.
>>>>>
>>>>> Please can somebody check and merge my chris/amissl-os4 branch
>>>>> into toolchains, and rebuild the ppc-amigaos toolchain?
>>>>> The equivalent chris/amissl-os4 branch can then be merged into
>>>>> NetSurf and the CI builds should be working again on OS4 beta.
>>>>>
>>>>> Thanks!
>>>>> Chris
>>>>>
>>>>>
>>>> hi chris, apologies for the delay. I applied a variant on your
>>>> changes to the toolchain (selecting the ssl provider suppresses
>>>> openssl so some changes you had were not required)
>>>>
>>>> I also updated the c-ares version which may help with the DNS
>>>> issues (maybe)
>>>>
>>>> You should be ok to merge any netsurf changes you want now
>>>>
>>>>
>>> Fab, thanks. I'll try with the new c-ares - it looked like the
>>> issue was that it didn't have the code for obtaining DNS servers in
>>> it (which I added), but it still didn't work. Maybe I did something
>>> wrong, I'll try again.
>>>
>>
>> Well, this is a bit weird. After all the updates I'm now getting
>> "unsupported protocol", even from the Curl executable. It shows http
>> and https in the list but neither are working :(
>>
>>
> I updated the SDK with up to date versions of most libraries
> (including libcurl) and re-built the CI toolchains. This was actually
> because I was hoping to do a release this month amd we were behind on
> security updates
>
> Maybe this will help? unsure why building libcurl with a different ssl
> library would cause the protocol failures you see though.
>

I was wondering if the updated libcurl was the issue. It isn't the SSL
library causing it as even non-SSL sites are returning the same error!

I'll investigate further.

[netsurf-dev] Re: Toolchain build for AmigaOS4

On 08/11/2024 10:48, Chris Young wrote:
>
>
>
> On 07/11/2024 10:56, Chris Young wrote:
>>
>>
>> On 06/11/2024 13:09, vincent sanders wrote:
>>>
>>> On 18/10/2024 15:09, Chris Young wrote:
>>>> Hi all
>>>>
>>>> I've had some issues this year with various things not working, and
>>>> tracked it down to the OS4 betas and (indirectly) the update of the
>>>> SDK. There were two problems;
>>>> 1. DNS resolution was not working. I fixed this at the start of
>>>> the year but after fixing (2) it broke again in a different way.
>>>> I've temporarily disabled the threaded resolver until I can find a
>>>> more permanent solution.
>>>> 2. SSL/TLS sites were not opening. I'm not sure why but it may be
>>>> the same problem as OS3 was having with OpenSSL. As AmiSSL is kept
>>>> up-to-date now, unlike when I started porting NetSurf, I've
>>>> switched the OS4 build over to using AmiSSL.
>>>>
>>>> Please can somebody check and merge my chris/amissl-os4 branch into
>>>> toolchains, and rebuild the ppc-amigaos toolchain?
>>>> The equivalent chris/amissl-os4 branch can then be merged into
>>>> NetSurf and the CI builds should be working again on OS4 beta.
>>>>
>>>> Thanks!
>>>> Chris
>>>>
>>>>
>>> hi chris, apologies for the delay. I applied a variant on your
>>> changes to the toolchain (selecting the ssl provider suppresses
>>> openssl so some changes you had were not required)
>>>
>>> I also updated the c-ares version which may help with the DNS issues
>>> (maybe)
>>>
>>> You should be ok to merge any netsurf changes you want now
>>>
>>>
>> Fab, thanks. I'll try with the new c-ares - it looked like the issue
>> was that it didn't have the code for obtaining DNS servers in it
>> (which I added), but it still didn't work. Maybe I did something
>> wrong, I'll try again.
>>
>
> Well, this is a bit weird. After all the updates I'm now getting
> "unsupported protocol", even from the Curl executable. It shows http
> and https in the list but neither are working :(
>
>
I updated the SDK with up to date versions of most libraries (including
libcurl) and re-built the CI toolchains. This was actually because I was
hoping to do a release this month amd we were behind on security updates

Maybe this will help? unsure why building libcurl with a different ssl
library would cause the protocol failures you see though.

[netsurf-dev] Re: Toolchain build for AmigaOS4

On 07/11/2024 10:56, Chris Young wrote:
>
>
> On 06/11/2024 13:09, vincent sanders wrote:
>>
>> On 18/10/2024 15:09, Chris Young wrote:
>>> Hi all
>>>
>>> I've had some issues this year with various things not working, and
>>> tracked it down to the OS4 betas and (indirectly) the update of the
>>> SDK. There were two problems;
>>> 1. DNS resolution was not working. I fixed this at the start of the
>>> year but after fixing (2) it broke again in a different way. I've
>>> temporarily disabled the threaded resolver until I can find a more
>>> permanent solution.
>>> 2. SSL/TLS sites were not opening. I'm not sure why but it may be
>>> the same problem as OS3 was having with OpenSSL. As AmiSSL is kept
>>> up-to-date now, unlike when I started porting NetSurf, I've switched
>>> the OS4 build over to using AmiSSL.
>>>
>>> Please can somebody check and merge my chris/amissl-os4 branch into
>>> toolchains, and rebuild the ppc-amigaos toolchain?
>>> The equivalent chris/amissl-os4 branch can then be merged into
>>> NetSurf and the CI builds should be working again on OS4 beta.
>>>
>>> Thanks!
>>> Chris
>>>
>>>
>> hi chris, apologies for the delay. I applied a variant on your
>> changes to the toolchain (selecting the ssl provider suppresses
>> openssl so some changes you had were not required)
>>
>> I also updated the c-ares version which may help with the DNS issues
>> (maybe)
>>
>> You should be ok to merge any netsurf changes you want now
>>
>>
> Fab, thanks. I'll try with the new c-ares - it looked like the issue
> was that it didn't have the code for obtaining DNS servers in it
> (which I added), but it still didn't work. Maybe I did something
> wrong, I'll try again.
>

Well, this is a bit weird. After all the updates I'm now getting
"unsupported protocol", even from the Curl executable. It shows http
and https in the list but neither are working :(

Thursday, 7 November 2024

[netsurf-dev] Re: Toolchain build for AmigaOS4

On 06/11/2024 13:09, vincent sanders wrote:
>
> On 18/10/2024 15:09, Chris Young wrote:
>> Hi all
>>
>> I've had some issues this year with various things not working, and
>> tracked it down to the OS4 betas and (indirectly) the update of the
>> SDK. There were two problems;
>> 1. DNS resolution was not working. I fixed this at the start of the
>> year but after fixing (2) it broke again in a different way. I've
>> temporarily disabled the threaded resolver until I can find a more
>> permanent solution.
>> 2. SSL/TLS sites were not opening. I'm not sure why but it may be
>> the same problem as OS3 was having with OpenSSL. As AmiSSL is kept
>> up-to-date now, unlike when I started porting NetSurf, I've switched
>> the OS4 build over to using AmiSSL.
>>
>> Please can somebody check and merge my chris/amissl-os4 branch into
>> toolchains, and rebuild the ppc-amigaos toolchain?
>> The equivalent chris/amissl-os4 branch can then be merged into
>> NetSurf and the CI builds should be working again on OS4 beta.
>>
>> Thanks!
>> Chris
>>
>>
> hi chris, apologies for the delay. I applied a variant on your changes
> to the toolchain (selecting the ssl provider suppresses openssl so
> some changes you had were not required)
>
> I also updated the c-ares version which may help with the DNS issues
> (maybe)
>
> You should be ok to merge any netsurf changes you want now
>
>
Fab, thanks. I'll try with the new c-ares - it looked like the issue
was that it didn't have the code for obtaining DNS servers in it (which
I added), but it still didn't work. Maybe I did something wrong, I'll
try again.

Chris

Wednesday, 6 November 2024

[netsurf-dev] Re: Toolchain build for AmigaOS4

On 18/10/2024 15:09, Chris Young wrote:
> Hi all
>
> I've had some issues this year with various things not working, and
> tracked it down to the OS4 betas and (indirectly) the update of the
> SDK. There were two problems;
> 1. DNS resolution was not working. I fixed this at the start of the
> year but after fixing (2) it broke again in a different way. I've
> temporarily disabled the threaded resolver until I can find a more
> permanent solution.
> 2. SSL/TLS sites were not opening. I'm not sure why but it may be the
> same problem as OS3 was having with OpenSSL. As AmiSSL is kept
> up-to-date now, unlike when I started porting NetSurf, I've switched
> the OS4 build over to using AmiSSL.
>
> Please can somebody check and merge my chris/amissl-os4 branch into
> toolchains, and rebuild the ppc-amigaos toolchain?
> The equivalent chris/amissl-os4 branch can then be merged into NetSurf
> and the CI builds should be working again on OS4 beta.
>
> Thanks!
> Chris
>
>
hi chris, apologies for the delay. I applied a variant on your changes
to the toolchain (selecting the ssl provider suppresses openssl so some
changes you had were not required)

I also updated the c-ares version which may help with the DNS issues (maybe)

You should be ok to merge any netsurf changes you want now

Wednesday, 30 October 2024

[netsurf-dev] Re: Pull request: CSS support for libsvgtiny

On 2024-10-23 19:01:26, Michael Drake wrote:
> On Tue, 22 Oct 2024 at 13:35, Michael Orlitzky <michael@orlitzky.com> wrote:
>
> > One small concern: since we are looking for indices and sheets that the
> > API has just told us exist... if get_sheet() or remove_sheet() fail, an
> > svgtiny_LIBCSS_ERROR doesn't really do it justice. Maybe an assert()
> > for these as well?
>
> Good idea, yes, an assert makes sense.

I just discovered that my MTA wasn't working and I don't remember if
my reply made it to the list, so just in case: this is done.

Tuesday, 29 October 2024

[netsurf-dev] [PATCH 3/3] Use dynamic path allocation in gtk and fb frontends

From: Olivier Valentin <valentio@free.fr>

---
frontends/framebuffer/fetch.c | 6 ++++--
frontends/gtk/fetch.c | 6 ++++--
frontends/gtk/gui.c | 9 ++++++---
utils/filepath.c | 35 +++++++++++++++++++++++++----------
utils/filepath.h | 14 +++++++++++++-
5 files changed, 52 insertions(+), 18 deletions(-)

diff --git a/frontends/framebuffer/fetch.c b/frontends/framebuffer/fetch.c
index f917bddf3..9d76f0818 100644
--- a/frontends/framebuffer/fetch.c
+++ b/frontends/framebuffer/fetch.c
@@ -48,14 +48,16 @@
*/
static nsurl *get_resource_url(const char *path)
{
- char buf[PATH_MAX];
+ char *buf;
nsurl *url = NULL;

if (strcmp(path, "favicon.ico") == 0)
path = "favicon.png";

- if (filepath_sfind(respaths, buf, PATH_MAX, path) == NSERROR_OK)
+ if (filepath_find(respaths, path, &buf) == NSERROR_OK) {
netsurf_path_to_nsurl(buf, &url);
+ free(buf);
+ }

return url;
}
diff --git a/frontends/gtk/fetch.c b/frontends/gtk/fetch.c
index 98682ffc4..91219ad96 100644
--- a/frontends/gtk/fetch.c
+++ b/frontends/gtk/fetch.c
@@ -250,15 +250,17 @@ const char *fetch_filetype(const char *unix_path)

static nsurl *nsgtk_get_resource_url(const char *path)
{
- char buf[PATH_MAX];
nsurl *url = NULL;

/* favicon.ico -> favicon.png */
if (strcmp(path, "favicon.ico") == 0) {
nsurl_create("resource:favicon.png", &url);
} else {
- if (filepath_sfind(respaths, buf, PATH_MAX, path) == NSERROR_OK)
+ char *buf;
+ if (filepath_find(respaths, path, &buf) == NSERROR_OK) {
netsurf_path_to_nsurl(buf, &url);
+ free(buf);
+ }
}

return url;
diff --git a/frontends/gtk/gui.c b/frontends/gtk/gui.c
index 2c5558ddb..725c55b2a 100644
--- a/frontends/gtk/gui.c
+++ b/frontends/gtk/gui.c
@@ -910,7 +910,7 @@ static nserror nsgtk_add_named_icons_to_theme(void)
*/
static nserror nsgtk_setup(int argc, char** argv, char **respath)
{
- char buf[PATH_MAX];
+ char *buf;
char *resource_filename;
char *addr = NULL;
nsurl *url;
@@ -987,8 +987,11 @@ static nserror nsgtk_setup(int argc, char** argv, char **respath)
.pma = true,
});

- filepath_sfinddef(respath, buf, PATH_MAX, "mime.types", "/etc/");
- gtk_fetch_filetype_init(buf);
+ res = filepath_finddef(respath, "mime.types", "/etc/", &buf);
+ if (res == NSERROR_OK) {
+ gtk_fetch_filetype_init(buf);
+ free(buf);
+ }

save_complete_init();

diff --git a/utils/filepath.c b/utils/filepath.c
index 82568e298..626dc2c89 100644
--- a/utils/filepath.c
+++ b/utils/filepath.c
@@ -160,29 +160,44 @@ nserror filepath_find(char **respathv, const char *filename, char **retstr)

/* exported interface documented in filepath.h */
nserror
-filepath_sfinddef(char **respathv,
- char *filepath,
- size_t size,
- const char *filename,
- const char *def)
+filepath_finddef(char **respathv,
+ const char *filename,
+ const char *def,
+ char **retstr)
{
- char *allocatedfilepath;
nserror ret;

- if ((respathv == NULL) || (respathv[0] == NULL) || (filepath == NULL))
+ if ((respathv == NULL) || (respathv[0] == NULL) || (retstr == NULL))
return NSERROR_INVALID;

- ret = filepath_find(respathv, filename, &allocatedfilepath);
+ ret = filepath_find(respathv, filename, retstr);

if ((ret != NSERROR_OK) && (def != NULL)) {
/* search failed, return the path specified */
if (def[0] == '~') {
- ret = filepath_printpath(&allocatedfilepath, "%s/%s/%s", getenv("HOME"), def + 1, filename);
+ ret = filepath_printpath(retstr, "%s/%s/%s", getenv("HOME"), def + 1, filename);
} else {
- ret = filepath_printpath(&allocatedfilepath, "%s/%s", def, filename);
+ ret = filepath_printpath(retstr, "%s/%s", def, filename);
}
}

+ return ret;
+}
+
+
+/* exported interface documented in filepath.h */
+nserror
+filepath_sfinddef(char **respathv,
+ char *filepath,
+ size_t size,
+ const char *filename,
+ const char *def)
+{
+ nserror ret;
+ char *allocatedfilepath;
+
+ ret = filepath_finddef(respathv, filename, def, &allocatedfilepath);
+
if (ret == NSERROR_OK) {
if (strlen(allocatedfilepath) >= size)
ret = NSERROR_NOSPACE;
diff --git a/utils/filepath.h b/utils/filepath.h
index c44f5af7e..f9316c997 100644
--- a/utils/filepath.h
+++ b/utils/filepath.h
@@ -68,12 +68,24 @@ nserror filepath_find(char **respathv, const char *filename, char **retstr);
/**
* Searches an array of resource paths for a file optionally forcing a default.
*
- * Similar to filepath_sfind except if no resource is found the default
+ * Similar to filepath_find except if no resource is found the default
* is used as an additional path element to search, if that still
* fails the returned path is set to the concatination of the default
* path and the filename.
*
* @param respathv The resource path vector to iterate.
+ * @param filename The filename of the resource to search for.
+ * @param def The default path to use.
+ * @param retstr The result string containing the path.
+ * @return NSERROR_OK on success or error code on failure.
+ */
+nserror filepath_finddef(char **respathv, const char *filename,
+ const char *def, char **retstr);
+
+/**
+ * Variant of filepath_finddef returning the result in a prealocated buffer.
+ *
+ * @param respathv The resource path vector to iterate.
* @param filepath The buffer to place the result in.
* @param size The size of the filepath buffer.
* @param filename The filename of the resource to search for.
--
2.45.2

[netsurf-dev] [PATCH 2/3] Turn every PATH_MAX usage into an explicit parameter.

From: Olivier Valentin <valentio@free.fr>

When static buffers are used, PATH_MAX becomes the size of the provided
buffer, and is passed as a parameters to the underlying implementation.
---
frontends/framebuffer/fetch.c | 2 +-
frontends/gtk/fetch.c | 2 +-
frontends/gtk/gui.c | 2 +-
frontends/monkey/fetch.c | 2 +-
frontends/monkey/main.c | 2 +-
frontends/windows/fetch.c | 2 +-
utils/filepath.c | 143 ++++++++++++++++++----------------
utils/filepath.h | 39 +++-------
8 files changed, 92 insertions(+), 102 deletions(-)

diff --git a/frontends/framebuffer/fetch.c b/frontends/framebuffer/fetch.c
index d58a82205..f917bddf3 100644
--- a/frontends/framebuffer/fetch.c
+++ b/frontends/framebuffer/fetch.c
@@ -54,7 +54,7 @@ static nsurl *get_resource_url(const char *path)
if (strcmp(path, "favicon.ico") == 0)
path = "favicon.png";

- if (filepath_sfind(respaths, buf, path) == NSERROR_OK)
+ if (filepath_sfind(respaths, buf, PATH_MAX, path) == NSERROR_OK)
netsurf_path_to_nsurl(buf, &url);

return url;
diff --git a/frontends/gtk/fetch.c b/frontends/gtk/fetch.c
index 36ee36c17..98682ffc4 100644
--- a/frontends/gtk/fetch.c
+++ b/frontends/gtk/fetch.c
@@ -257,7 +257,7 @@ static nsurl *nsgtk_get_resource_url(const char *path)
if (strcmp(path, "favicon.ico") == 0) {
nsurl_create("resource:favicon.png", &url);
} else {
- if (filepath_sfind(respaths, buf, path) == NSERROR_OK)
+ if (filepath_sfind(respaths, buf, PATH_MAX, path) == NSERROR_OK)
netsurf_path_to_nsurl(buf, &url);
}

diff --git a/frontends/gtk/gui.c b/frontends/gtk/gui.c
index fa0c4063b..2c5558ddb 100644
--- a/frontends/gtk/gui.c
+++ b/frontends/gtk/gui.c
@@ -987,7 +987,7 @@ static nserror nsgtk_setup(int argc, char** argv, char **respath)
.pma = true,
});

- filepath_sfinddef(respath, buf, "mime.types", "/etc/");
+ filepath_sfinddef(respath, buf, PATH_MAX, "mime.types", "/etc/");
gtk_fetch_filetype_init(buf);

save_complete_init();
diff --git a/frontends/monkey/fetch.c b/frontends/monkey/fetch.c
index 96ba28d67..f6d554915 100644
--- a/frontends/monkey/fetch.c
+++ b/frontends/monkey/fetch.c
@@ -39,7 +39,7 @@ static nsurl *gui_get_resource_url(const char *path)
char buf[PATH_MAX];
nsurl *url = NULL;

- if (filepath_sfind(respaths, buf, path) == NSERROR_OK)
+ if (filepath_sfind(respaths, buf, PATH_MAX, path) == NSERROR_OK)
netsurf_path_to_nsurl(buf, &url);

return url;
diff --git a/frontends/monkey/main.c b/frontends/monkey/main.c
index 2d6c1bd1b..35af25235 100644
--- a/frontends/monkey/main.c
+++ b/frontends/monkey/main.c
@@ -452,7 +452,7 @@ main(int argc, char **argv)
die("NetSurf failed to initialise");
}

- filepath_sfinddef(respaths, buf, "mime.types", "/etc/");
+ filepath_sfinddef(respaths, buf, PATH_MAX, "mime.types", "/etc/");
monkey_fetch_filetype_init(buf);

urldb_load(nsoption_charp(url_file));
diff --git a/frontends/windows/fetch.c b/frontends/windows/fetch.c
index cad99446b..bb5290e59 100644
--- a/frontends/windows/fetch.c
+++ b/frontends/windows/fetch.c
@@ -79,7 +79,7 @@ static nsurl *nsw32_get_resource_url(const char *path)
char buf[PATH_MAX];
nsurl *url = NULL;

- if (filepath_sfind(G_resource_pathv, buf, path) == NSERROR_OK)
+ if (filepath_sfind(G_resource_pathv, buf, PATH_MAX, path) == NSERROR_OK)
netsurf_path_to_nsurl(buf, &url);

return url;
diff --git a/utils/filepath.c b/utils/filepath.c
index 5cfcba548..82568e298 100644
--- a/utils/filepath.c
+++ b/utils/filepath.c
@@ -33,7 +33,6 @@
#include <unistd.h>
#include <string.h>

-#include "utils/dirent.h" /** \todo why is this necessary for atari to get PATH_MAX and is there a better way */
#include "utils/utils.h"
#include "utils/config.h"
#include "utils/filepath.h"
@@ -41,119 +40,121 @@
/** maximum number of elements in the resource vector */
#define MAX_RESPATH 128

-/* exported interface documented in filepath.h */
-nserror filepath_vsfindfile(char *str, const char *format, va_list ap)
+static nserror filepath_vprintpath(const char *format, va_list ap, char **retstr)
{
- char *realpathname;
- char *pathname;
- int len;
+ char *pathname = NULL;
+ int len = 0;

- pathname = malloc(PATH_MAX);
- if (pathname == NULL)
- return NSERROR_NOMEM; /* unable to allocate memory */
+ do {
+ len = vsnprintf(pathname, len, format, ap);
+ if (len <= 0)
+ return NSERROR_INVALID;

- len = vsnprintf(pathname, PATH_MAX, format, ap);
+ if (pathname != NULL) break;

- if ((len < 0) || (len >= PATH_MAX)) {
- /* error or output exceeded PATH_MAX length so
- * operation is doomed to fail.
- */
- free(pathname);
- NSERROR_BAD_SIZE;
- }
+ pathname = malloc(len + 1);
+ } while (pathname != NULL);

- realpathname = realpath(pathname, str);
+ if (pathname == NULL)
+ return NSERROR_NOMEM; /* unable to allocate memory */

- free(pathname);
+ *retstr = realpath(pathname, NULL);

- if (realpathname != NULL) {
- /* sucessfully expanded pathname */
- if (access(realpathname, R_OK) != 0) {
- /* unable to read the file */
- return NSERROR_NOT_FOUND;
- }
- }
+ if (*retstr == NULL)
+ *retstr = pathname;
+ else
+ free(pathname);

return NSERROR_OK;
}

-
-/* exported interface documented in filepath.h */
-nserror filepath_sfindfile(char *str, const char *format, ...)
-{
- va_list ap;
+static nserror filepath_printpath(char **retstr, const char *format, ...) {
nserror ret;
+ va_list ap;

va_start(ap, format);
- ret = filepath_vsfindfile(str, format, ap);
+ ret = filepath_vprintpath(format, ap, retstr);
va_end(ap);

return ret;
}

-
/* exported interface documented in filepath.h */
nserror filepath_findfile(char **retstr, const char *format, ...)
{
nserror ret;
va_list ap;

- *retstr = malloc(PATH_MAX);
- if (*retstr == NULL)
- return NSERROR_NOMEM; /* unable to allocate memory */
-
va_start(ap, format);
- ret = filepath_vsfindfile(*retstr, format, ap);
+ ret = filepath_vprintpath(format, ap, retstr);
va_end(ap);

- if (ret != NSERROR_OK) {
- free(*retstr);
- *retstr = NULL;
+ if (ret == NSERROR_OK) {
+ if (access(*retstr, F_OK) != 0) {
+ /* File not found */
+ free(*retstr);
+ *retstr = NULL;
+
+ return NSERROR_NOT_FOUND;
+ }
+
+ if (access(*retstr, R_OK) != 0) {
+ /* unable to read the file */
+ free(*retstr);
+ *retstr = NULL;
+
+ return NSERROR_PERMISSION;
+ }
}

return ret;
}

/* exported interface documented in filepath.h */
-nserror filepath_sfind(char **respathv, char *filepath, const char *filename)
+nserror filepath_sfind(char **respathv, char *filepath, size_t size, const char *filename)
{
- int respathc = 0;
+ nserror ret;
+ char *allocatedfilepath;

- if ((respathv == NULL) || (respathv[0] == NULL) || (filepath == NULL))
+ if (filepath == NULL || size == 0)
return NSERROR_INVALID;

- while (respathv[respathc] != NULL) {
- if (filepath_sfindfile(filepath, "%s/%s", respathv[respathc], filename) == NSERROR_OK) {
- return NSERROR_OK;
- }
+ ret = filepath_find(respathv, filename, &allocatedfilepath);
+ if (ret == NSERROR_OK) {
+ if (strlen(allocatedfilepath) >= size)
+ ret = NSERROR_NOSPACE;

- respathc++;
+ strncpy(filepath, allocatedfilepath, size);
+ filepath[size - 1] = '\0';
+
+ free(allocatedfilepath);
}

- return NSERROR_NOT_FOUND;
+ return ret;
}

/* exported interface documented in filepath.h */
nserror filepath_find(char **respathv, const char *filename, char **retstr)
{
- nserror ret;
+ int respathc = 0;

if ((respathv == NULL) || (respathv[0] == NULL))
return NSERROR_INVALID;

- *retstr = malloc(PATH_MAX);
- if (*retstr == NULL)
- return NSERROR_NOMEM;
+ while (respathv[respathc] != NULL) {
+ nserror ret;

- ret = filepath_sfind(respathv, *retstr, filename);
+ ret = filepath_findfile(retstr, "%s/%s", respathv[respathc], filename);
+ if (ret == NSERROR_OK)
+ return NSERROR_OK;
+ if (ret != NSERROR_NOT_FOUND)
+ return ret;

- if (ret == NSERROR_OK) {
- free(*retstr);
- *retstr = NULL;
+ respathc++;
}

- return ret;
+ return NSERROR_NOT_FOUND;
}

@@ -161,29 +162,37 @@ nserror filepath_find(char **respathv, const char *filename, char **retstr)
nserror
filepath_sfinddef(char **respathv,
char *filepath,
+ size_t size,
const char *filename,
const char *def)
{
- char t[PATH_MAX];
+ char *allocatedfilepath;
nserror ret;

if ((respathv == NULL) || (respathv[0] == NULL) || (filepath == NULL))
return NSERROR_INVALID;

- ret = filepath_sfind(respathv, filepath, filename);
+ ret = filepath_find(respathv, filename, &allocatedfilepath);

if ((ret != NSERROR_OK) && (def != NULL)) {
/* search failed, return the path specified */
- ret = NSERROR_OK;
if (def[0] == '~') {
- snprintf(t, PATH_MAX, "%s/%s/%s", getenv("HOME"), def + 1, filename);
+ ret = filepath_printpath(&allocatedfilepath, "%s/%s/%s", getenv("HOME"), def + 1, filename);
} else {
- snprintf(t, PATH_MAX, "%s/%s", def, filename);
- }
- if (realpath(t, filepath) == NULL) {
- strncpy(filepath, t, PATH_MAX);
+ ret = filepath_printpath(&allocatedfilepath, "%s/%s", def, filename);
}
}
+
+ if (ret == NSERROR_OK) {
+ if (strlen(allocatedfilepath) >= size)
+ ret = NSERROR_NOSPACE;
+
+ strncpy(filepath, allocatedfilepath, size);
+ filepath[size - 1] = '\0';
+
+ free(allocatedfilepath);
+ }
+
return ret;
}

diff --git a/utils/filepath.h b/utils/filepath.h
index e87437d90..c44f5af7e 100644
--- a/utils/filepath.h
+++ b/utils/filepath.h
@@ -32,32 +32,13 @@
* Create a normalised file name.
*
* If the file described by the format exists and is accessible the
- * normalised path is placed in str and a pointer to str returned
- * otherwise NULL is returned. The string in str is always modified.
+ * normalised path is placed in retstr. The returned string is allocated
+ * and it is the responsibility of the caller to free it via free().
*
- * @param str A buffer to contain the normalised file name must be at
- * least PATH_MAX bytes long.
+ * @param retstr A pointer the the allocated result string.
* @param format A printf format for the filename.
- * @param ap The list of arguments for the format.
* @return NSERROR_OK on success or error code on failure.
*/
-nserror filepath_vsfindfile(char *str, const char *format, va_list ap);
-
-
-/**
- * Create a normalised file name.
- *
- * Similar to vsfindfile but takes variadic (printf like) parameters
- */
-nserror filepath_sfindfile(char *str, const char *format, ...);
-
-
-/**
- * Create a normalised file name.
- *
- * Similar to sfindfile but allocates its own storage for the
- * returned string in retstr. The caller must free this storage.
- */
nserror filepath_findfile(char **retstr, const char *format, ...);

@@ -65,22 +46,21 @@ nserror filepath_findfile(char **retstr, const char *format, ...);
* Searches an array of resource paths for a file.
*
* Iterates through a vector of resource paths and returns the
- * normalised file name of the first acessible file or NULL if no file
- * can be found in any of the resource paths.
+ * normalised file name of the first acessible file.
*
* @param respathv The resource path vector to iterate.
* @param filepath The buffer to place the result in.
* @param filename The filename of the resource to search for.
* @return NSERROR_OK on success or error code on failure.
*/
-nserror filepath_sfind(char **respathv, char *filepath, const char *filename);
+nserror filepath_sfind(char **respathv, char *filepath, size_t size, const char *filename);

/**
* Searches an array of resource paths for a file.
*
* Similar to filepath_sfind except it allocates its own storage for
- * the returned string. The caller must free this sorage.
+ * the returned string. The caller must free this storage.
*/
nserror filepath_find(char **respathv, const char *filename, char **retstr);

@@ -94,13 +74,14 @@ nserror filepath_find(char **respathv, const char *filename, char **retstr);
* path and the filename.
*
* @param respathv The resource path vector to iterate.
- * @param filepath The buffer to place the result in. Must have space for PATH_MAX bytes.
+ * @param filepath The buffer to place the result in.
+ * @param size The size of the filepath buffer.
* @param filename The filename of the resource to search for.
* @param def The default path to use
* @return NSERROR_OK on success or error code on failure.
*/
-nserror filepath_sfinddef(char **respathv, char *filepath, const char *filename,
- const char *def);
+nserror filepath_sfinddef(char **respathv, char *filepath, size_t size,
+ const char *filename, const char *def);

/**
--
2.45.2

[netsurf-dev] [PATCH 1/3] Return nserror in most filepath utility functions

From: Olivier Valentin <valentio@free.fr>

---
frontends/framebuffer/fetch.c | 3 +-
frontends/framebuffer/font_freetype.c | 4 +-
frontends/framebuffer/gui.c | 4 +-
frontends/gtk/fetch.c | 3 +-
frontends/gtk/gui.c | 2 +-
frontends/gtk/resources.c | 3 +-
frontends/monkey/fetch.c | 3 +-
frontends/monkey/main.c | 4 +-
frontends/windows/fetch.c | 3 +-
frontends/windows/main.c | 11 ++--
utils/filepath.c | 75 +++++++++++++++------------
utils/filepath.h | 35 ++++++-------
12 files changed, 78 insertions(+), 72 deletions(-)

diff --git a/frontends/framebuffer/fetch.c b/frontends/framebuffer/fetch.c
index 23cbb4f21..d58a82205 100644
--- a/frontends/framebuffer/fetch.c
+++ b/frontends/framebuffer/fetch.c
@@ -54,7 +54,8 @@ static nsurl *get_resource_url(const char *path)
if (strcmp(path, "favicon.ico") == 0)
path = "favicon.png";

- netsurf_path_to_nsurl(filepath_sfind(respaths, buf, path), &url);
+ if (filepath_sfind(respaths, buf, path) == NSERROR_OK)
+ netsurf_path_to_nsurl(buf, &url);

return url;
}
diff --git a/frontends/framebuffer/font_freetype.c b/frontends/framebuffer/font_freetype.c
index 3912821f7..0c43a95ee 100644
--- a/frontends/framebuffer/font_freetype.c
+++ b/frontends/framebuffer/font_freetype.c
@@ -120,15 +120,13 @@ fb_new_face(const char *option, const char *resname, const char *fontname)
fb_faceid_t *newf;
FT_Error error;
FT_Face aface;
- char buf[PATH_MAX];

newf = calloc(1, sizeof(fb_faceid_t));

if (option != NULL) {
newf->fontfile = strdup(option);
} else {
- filepath_sfind(respaths, buf, fontname);
- newf->fontfile = strdup(buf);
+ filepath_find(respaths, fontname, &newf->fontfile);
}

error = FTC_Manager_LookupFace(ft_cmanager, (FTC_FaceID)newf, &aface);
diff --git a/frontends/framebuffer/gui.c b/frontends/framebuffer/gui.c
index f0a8f5e58..74768c224 100644
--- a/frontends/framebuffer/gui.c
+++ b/frontends/framebuffer/gui.c
@@ -2214,13 +2214,13 @@ main(int argc, char** argv)
if (ret != NSERROR_OK) {
die("Options failed to initialise");
}
- options = filepath_find(respaths, "Choices");
+ filepath_find(respaths, "Choices", &options);
nsoption_read(options, nsoptions);
free(options);
nsoption_commandline(&argc, argv, nsoptions);

/* message init */
- messages = filepath_find(respaths, "Messages");
+ filepath_find(respaths, "Messages", &messages);
ret = messages_add_from_file(messages);
free(messages);
if (ret != NSERROR_OK) {
diff --git a/frontends/gtk/fetch.c b/frontends/gtk/fetch.c
index d77073a63..36ee36c17 100644
--- a/frontends/gtk/fetch.c
+++ b/frontends/gtk/fetch.c
@@ -257,7 +257,8 @@ static nsurl *nsgtk_get_resource_url(const char *path)
if (strcmp(path, "favicon.ico") == 0) {
nsurl_create("resource:favicon.png", &url);
} else {
- netsurf_path_to_nsurl(filepath_sfind(respaths, buf, path), &url);
+ if (filepath_sfind(respaths, buf, path) == NSERROR_OK)
+ netsurf_path_to_nsurl(buf, &url);
}

return url;
diff --git a/frontends/gtk/gui.c b/frontends/gtk/gui.c
index 644237e09..fa0c4063b 100644
--- a/frontends/gtk/gui.c
+++ b/frontends/gtk/gui.c
@@ -942,7 +942,7 @@ static nserror nsgtk_setup(int argc, char** argv, char **respath)
}

/* Search engine sources */
- resource_filename = filepath_find(respath, "SearchEngines");
+ filepath_find(respath, "SearchEngines", &resource_filename);
search_web_init(resource_filename);
if (resource_filename != NULL) {
NSLOG(netsurf, INFO, "Using '%s' as Search Engines file",
diff --git a/frontends/gtk/resources.c b/frontends/gtk/resources.c
index fc17f7418..425517b53 100644
--- a/frontends/gtk/resources.c
+++ b/frontends/gtk/resources.c
@@ -246,8 +246,7 @@ init_resource(char **respath, struct nsgtk_resource_s *resource)

[netsurf-dev] going dynamic with path strings

Hello !

I would like to restore NetSurf in Debian GNU/Hurd, but PATH_MAX is not
defined there. As discussed on IRC, using dynamic allocation for paths
would be a desirable solution, and it is the POSIX way.

Here are the proposed steps in detail :
- Have most of the file-path functions return nserror, as is the
convention in the codebase.
- Stop considering PATH_MAX as an implicit length for static buffers,
and provide the sizes as a parameter instead.
- Switch to dynamic allocation for GTK and framebuffer frontends. Those
are the ones built by Debian.

We could extend this to all the frontends, but let's see how things
go with this series first.

Thanks,

Olivier

Wednesday, 23 October 2024

[netsurf-dev] Re: Pull request: CSS support for libsvgtiny

On Tue, 22 Oct 2024 at 13:35, Michael Orlitzky <michael@orlitzky.com> wrote:

> One small concern: since we are looking for indices and sheets that the
> API has just told us exist... if get_sheet() or remove_sheet() fail, an
> svgtiny_LIBCSS_ERROR doesn't really do it justice. Maybe an assert()
> for these as well?

Good idea, yes, an assert makes sense.

Tuesday, 22 October 2024

[netsurf-users] Re: Netsurf CSS support.

On 21 Oct, Drag N Drop wrote in message
<5bb354af20dragdrop@dragdrop.co.uk>:

> I think the advice by Michael to use 'flex' was the magic search word
> allowing me to find more info starting with
> www.w3schools.com/css3_flexbox.asp. Although their tutorial page renders
> ok in Netsurf their HTML example code isn't helpful on any platform. Ended
> up examining the source code which at least is helping my learning.

For what it's worth, I always seem to end up back here when doing flexbox
stuff:

https://css-tricks.com/snippets/css/a-guide-to-flexbox/

--
Steve Fryatt - Leeds, England

http://www.stevefryatt.org.uk/

[netsurf-dev] Re: Pull request: CSS support for libsvgtiny

On Mon, 2024-10-21 at 20:36 +0100, Michael Drake wrote:
>
> This fixes it for DOM_NO_MEM_ERR, but in general I think the
> best approach is if a function returns an error code, we should
> handle any non-successful value.

No problem, I changed it.

> However I'm a bit uncomfortable about leaving the selection
> context with dangling sheet pointers, even if we are about to
> destroy it.... Also the error handling is not correct...
>
> I'd rather do something like this (untested):

Agreed, done.

One small concern: since we are looking for indices and sheets that the
API has just told us exist... if get_sheet() or remove_sheet() fail, an
svgtiny_LIBCSS_ERROR doesn't really do it justice. Maybe an assert()
for these as well?

> For this commit:
>
> https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=commit;h=29e40ed74de9f729755aa45459ff4f770b5be9ca
>
> I don't think that's the right fix. The real issue is that
> `_node_has_attribute_substring` has an early `return CSS_OK`
> (substrlen == 0) which doesn't assign a result to `*match`.
>

You're right, the other early return from that function is already
caught. Fixed!

Monday, 21 October 2024

[netsurf-dev] Re: Pull request: CSS support for libsvgtiny

On Mon, 21 Oct 2024 at 00:08, Michael Orlitzky <michael@orlitzky.com> wrote:
>
> On Sat, 2024-10-19 at 16:28 +0100, Michael Drake wrote:

> > https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=blob;f=src/svgtiny_css.c;h=96991ddfcfcfe4684b9411709c44367b4651fa92;hb=refs/heads/libcss#l1929

> I fixed the dom_string leak in one commit, and then dropped the
> comparison entirely (option 2) in another. Unless you are dying to
> revamp this API, the first option seems like a lot of work for little
> benefit.

Agreed!

> > There's also a comment saying:
> >
> > > dom_node_set_user_data() always returns DOM_NO_ERR
> >
> > However, it can return `DOM_NO_MEM_ERR` when malloc fails.
>
> I should know better, but in my defence, the documentation says it
> always returns DOM_NO_ERR :)

Whoops! :D

> This is now fixed.

https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=blobdiff;f=src/svgtiny_css.c;h=4856ac3f7bdefe4856f4bbf49415b6bf2c796415;hp=7d52ffa14df7cc11e808df741daa0f3f3db73826;hb=051ee66c3445a9071db366ed55cfd606cb579ec7;hpb=fe0f044f15984ebb02e017ab1b691178175c4b78

This fixes it for DOM_NO_MEM_ERR, but in general I think the
best approach is if a function returns an error code, we should
handle any non-successful value.

So it could be something like:

if (err == DOM_NO_MEM_ERR) {
return CSS_NOMEM;
} else if (err != DOM_NO_ERR) {
return CSS_INVALID;
}

Because if the implementation of a function ever changes, adding
a new type of error, we don't want to be checking every call site
in every project using the library!

> > In general there are a lot of ignored return values in the patches.
> >
> > https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=blob;f=src/svgtiny.c;h=9a6709a7e0069aad5290d43ebad3dcc9067d58a3;hb=refs/heads/libcss#l978

> Yeah I don't know how I overlooked this. I've modified all of the error
> paths to clean up the select results. I don't think the missing call to
> svgtiny_cleanup_state_local() was my fault but I see no reason why it
> shouldn't be there, so I've added that too.

Thanks, that looks great now!

> > https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=blob;f=src/svgtiny.c;h=9a6709a7e0069aad5290d43ebad3dcc9067d58a3;hb=refs/heads/libcss#l812

[snip]

> > So the sheet is leaked at the end of `svgtiny_parse_style_element`.

> 2. Just destroy whatever sheets are in the context object when it's
> time to clean up the context.
>
> I implemented the second option, but it
> feels a little dirty because you have to un-const the sheet references.

I agree with doing it this way, and you've written a good comment to
explain the dirtiness.

However I'm a bit uncomfortable about leaving the selection
context with dangling sheet pointers, even if we are about to
destroy it. E.g. someone might change the context destroy
code to log stuff from its sheets for debug.

Also the error handling is not correct. If `css_select_ctx_get_sheet`
fails, it will try to destroy whatever is pointed to by sheet anyway,
potentially causing a double-free or free of some random uninitialised
pointer.

I'd rather do something like this (untested):

for (i = 0; i < n_sheets; i++) {
/* Remove them in reverse order. */
css_code = css_select_ctx_get_sheet(state.select_ctx,
n_sheets - 1 - i, &sheet);
if (css_code != CSS_OK) {
if (code == svgtiny_OK) {
code = svgtiny_LIBCSS_ERROR;
}
continue;
}
css_code = css_select_ctx_remove_sheet(state.select_ctx, sheet);
if (css_code != CSS_OK) {
if (code == svgtiny_OK) {
code = svgtiny_LIBCSS_ERROR;
}
continue;
}
css_code = css_stylesheet_destroy((css_stylesheet*)sheet);
if (css_code != CSS_OK) {
if (code == svgtiny_OK) {
code = svgtiny_LIBCSS_ERROR;
}
}
}

Note the added `continue`s.

> Unrelated to these changes, I also noticed that GCC's -fanalyzer finds
> some problems with libsvgtiny,

Oh, thanks for letting us know! I'll look into it soon.

> and building against libwapcaplet fails
> with clang-19 due to a bunch of these:

Yeah, I think we'll need to drop `-pedantic` from the library CFLAGS.
We don't use it with our other libraries.

For this commit:

https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=commit;h=29e40ed74de9f729755aa45459ff4f770b5be9ca

I don't think that's the right fix. The real issue is that
`_node_has_attribute_substring` has an early `return CSS_OK`
(substrlen == 0) which doesn't assign a result to `*match`.

Best regards,
Michael

[netsurf-users] Re: Netsurf CSS support.

Hi all,

Thanks very much for all the replies, I think the advice by Michael to use
'flex' was the magic search word allowing me to find more info starting
with www.w3schools.com/css3_flexbox.asp. Although their tutorial page
renders ok in Netsurf their HTML example code isn't helpful on any
platform. Ended up examining the source code which at least is helping my
learning.

thanks,

Chris.

--
--
Chris Dewhurst, Editor, Drag N Drop for RISC OS
editor@dragdrop.co.uk www.dragdrop.co.uk

Sunday, 20 October 2024

[netsurf-dev] Re: Pull request: CSS support for libsvgtiny

On Sat, 2024-10-19 at 16:28 +0100, Michael Drake wrote:
>
> Sorry it's taken so long. I've finally had another look.

No problem, thanks for taking the time!

> https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=blob;f=src/svgtiny_css.c;h=96991ddfcfcfe4684b9411709c44367b4651fa92;hb=refs/heads/libcss#l1929
>
> In `svgtiny_dom_user_data_handler` str is leaked. There is a comment
> there is a comment there that says that there is no way to access the
> `state->interned_userdata_key`.
>
> There are two options here:
>
> 1. We could change the `dom_node_set_user_data` function to take
> a `void *pw` alongside the `handler` callback, and change the hander
> prototype to add an argument to pass back the user `pw`.
> 2. We can simply drop the string comparison here. The only user data
> we add that uses that callback is the libcss_node_data. If we added
> any other node data, we could add alongside a different callback
> function.

I fixed the dom_string leak in one commit, and then dropped the
comparison entirely (option 2) in another. Unless you are dying to
revamp this API, the first option seems like a lot of work for little
benefit.

> There's also a comment saying:
>
> > dom_node_set_user_data() always returns DOM_NO_ERR
>
> However, it can return `DOM_NO_MEM_ERR` when malloc fails.

I should know better, but in my defense, the documentation says it
always returns DOM_NO_ERR :)

This is now fixed.

> In general there are a lot of ignored return values in the patches.
>
> https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=blob;f=src/svgtiny.c;h=9a6709a7e0069aad5290d43ebad3dcc9067d58a3;hb=refs/heads/libcss#l978
>
> In `svgtiny_parse_svg`, it calls `svgtiny_parse_styles` which returns
> the css_select_results. These are leaked on all the error paths in
> `svgtiny_parse_svg` and I think at least one of the early returns is
> missing a call to `svgtiny_cleanup_state_local`.

Yeah I don't know how I overlooked this. I've modified all of the error
paths to clean up the select results. I don't think the missing call to
svgtiny_cleanup_state_local() was my fault but I see no reason why it
shouldn't be there, so I've added that too.

We were also leaking some DOM nodes that I've moved under the
"cleanup:" label.

> https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=blob;f=src/svgtiny.c;h=9a6709a7e0069aad5290d43ebad3dcc9067d58a3;hb=refs/heads/libcss#l812
>
> In `svgtiny_parse_style_element`, the `svgtiny_create_stylesheet`
> function is called, which returns a css_stylesheet pointer. Data is
> added to the sheet and then the sheet is given to
> `css_select_ctx_append_sheet`.
>
> However, `css_select_ctx_append_sheet` does not take ownership
> of the sheet (it takes it with the const qualifier).
>
> So the sheet is leaked at the end of `svgtiny_parse_style_element`.

Indeed. We can't destroy the sheet yet at the end of that function, so
I saw two options:

1. Keep track of the allocated sheets in the global "state" that we
pass around, so that they can eventually be cleaned up.
2. Just destroy whatever sheets are in the context object when it's
time to clean up the context.

Option 1 seems pointlessly wasteful, but option 2 only works if you
know that nobody else is going to be adding or removing sheets from the
context object (true for now). I implemented the second option, but it
feels a little dirty because you have to un-const the sheet references.
I'm happy to change it if you prefer the first option.

Unrelated to these changes, I also noticed that GCC's -fanalyzer finds
some problems with libsvgtiny, and building against libwapcaplet fails
with clang-19 due to a bunch of these:

src/svgtiny_css.c:121:9: error: use of GNU statement expression
extension from macro expansion [-Werror,-Wgnu-statement-expression-
from-macro-expansion]
121 | *abs = lwc_string_ref(rel);
| ^
/usr/include/libwapcaplet/libwapcaplet.h:137:30: note: expanded from
macro 'lwc_string_ref'
137 | #define lwc_string_ref(str) ({lwc_string *__lwc_s = (str);
assert(__lwc_s != NULL); __lwc_s->refcnt++; __lwc_s;})
| ^

Sunday, 22 December 2024

Friday, 15 November 2024

Friday, 8 November 2024

Thursday, 7 November 2024

Wednesday, 6 November 2024

Wednesday, 30 October 2024

Tuesday, 29 October 2024

Wednesday, 23 October 2024

Tuesday, 22 October 2024

Monday, 21 October 2024

Sunday, 20 October 2024

Saturday, 19 October 2024

Friday, 18 October 2024

Thursday, 17 October 2024

Wednesday, 16 October 2024

Tuesday, 15 October 2024

Monday, 14 October 2024

Sunday, 13 October 2024

Sunday, 29 September 2024

Sunday, 22 September 2024