[GH-ISSUE #61] Potential problem with ownership of page breaks #54

New issue

Closed

opened 2026-05-05 11:32:11 -06:00 by gitea-mirror · 4 comments

gitea-mirror commented

2026-05-05 11:32:11 -06:00

Owner

Originally created by @utelle on GitHub (Jul 4, 2016).
Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/61

Originally assigned to: @jmcnamara on GitHub.

The worksheet functions worksheet_set_h_pagebreaks and worksheet_set_v_pagebreaks take an lxw_row_t resp lxw_col_t array. However, those functions don't take ownership of the arrays, they just set a reference to them in the worksheet data structure. Thus, the programmer has to ensure that the page break array doesn't go out of scope while the worksheet is processed. That is, the following simple example, where the page break array is declared within a function and goes out of scope as soon as the function is left, would show unpredictable behaviour (or might even crash) later on:

void setHorizontalPageBreaks(lxw_worksheet* worksheet)
{
  lxw_row_t breaks[] = {20, 0};
  worksheet_set_h_pagebreaks(worksheet, breaks);
}

I think it would be better if the worksheet would take ownership of the arrays by making copies of them and storing the copies internally (just as is done for strings which are duplicated using strdup).

You could argue "just don't declare these arrays in a way that they could go out of scope". However, for the C++ wrapper classes I currently implement this imposes a real problem. The page break array is passed to the wrapper in a different data structure and the lxw_row_t array would be created on the fly, but this would not work at the moment. The wrapper class would have to take measures to ensure the lifetime of the created arrays somehow, i.e. persist the arrays within the wrapper class. Not impossible, but cumbersome, and probably error-prone.

Originally created by @utelle on GitHub (Jul 4, 2016). Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/61 Originally assigned to: @jmcnamara on GitHub. The `worksheet` functions `worksheet_set_h_pagebreaks` and `worksheet_set_v_pagebreaks` take an `lxw_row_t` resp `lxw_col_t` array. However, those functions don't take ownership of the arrays, they just set a reference to them in the `worksheet` data structure. Thus, the programmer has to ensure that the page break array doesn't go out of scope while the `worksheet` is processed. That is, the following simple example, where the page break array is declared within a function and goes out of scope as soon as the function is left, would show unpredictable behaviour (or might even crash) later on: ``` void setHorizontalPageBreaks(lxw_worksheet* worksheet) { lxw_row_t breaks[] = {20, 0}; worksheet_set_h_pagebreaks(worksheet, breaks); } ``` I think it would be better if the `worksheet` would take ownership of the arrays by making copies of them and storing the copies internally (just as is done for strings which are duplicated using `strdup`). You could argue "just don't declare these arrays in a way that they could go out of scope". However, for the C++ wrapper classes I currently implement this imposes a real problem. The page break array is passed to the wrapper in a different data structure and the `lxw_row_t` array would be created on the fly, but this would not work at the moment. The wrapper class would have to take measures to ensure the lifetime of the created arrays somehow, i.e. persist the arrays within the wrapper class. Not impossible, but cumbersome, and probably error-prone.

gitea-mirror

2026-05-05 11:32:11 -06:00

closed this issue
added the
feature request

bug

in progress

ready to close
labels

gitea-mirror commented

2026-05-05 11:32:20 -06:00

Author

Owner

@jmcnamara commented on GitHub (Jul 4, 2016):

You could argue "just don't declare these arrays in a way that they could go out of scope".

Actually, I wouldn't argue that. :-)

In almost all cases I try to copy the user data in case it goes out of scope. However, I missed this one (and possibly others).

I'll fix it.

@jmcnamara commented on GitHub (Jul 4, 2016): > You could argue "just don't declare these arrays in a way that they could go out of scope". Actually, I wouldn't argue that. :-) In almost all cases I try to copy the user data in case it goes out of scope. However, I missed this one (and possibly others). I'll fix it.

gitea-mirror commented

2026-05-05 11:32:23 -06:00

Author

Owner

@utelle commented on GitHub (Jul 4, 2016):

Thanks.

I will continue to report issues, if I come across them.

@utelle commented on GitHub (Jul 4, 2016): Thanks. I will continue to report issues, if I come across them.

gitea-mirror commented

2026-05-05 11:32:26 -06:00

Author

Owner