Fix UUID generation with SSDP disabled #1119

lars18th · 2023-06-12T09:45:03Z

This fixes the empty UUID when the SSDP is disabled with the command line parameter -G, as described in #1078.

* Handle CWs out of cycle * Change CW time after calculate expire time

… and only CAIDs accepted by the CAM

Eventually all parameter parsing should be factored out to separate functions like this so they can be tested in isolation

- specifying "~" made the hostname contain "~" if an offset was specified - specifying an offset truncated the hostname

…-dev

Fixes: stream.c:611:5: warning: implicit conversion from 'int' to 'char' changes value from 128 to -128 [-Wconstant-conversion] copy16(rtp_buf, len + 0, (uint16_t)0x8021); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../src/minisatip.h:34:27: note: expanded from macro 'copy16' a[i] = ((v) >> 8) & 0xFF; \ ~ ~~~~~~~~~~~^~~~~~ 1 warning generated.

Some SAT>IP clients send the parameter "bw" (DVB-T/C) in a incorrect range. The specifications indicate that acceptable values are: "5”, ”6”, “7”, “8", “10”, “1.712”. Therefore values like "bw=8000" or "bw=8000000" are incorrect. But we can handle them. In fact previously these requests will generate an overflow internaly in the value but the request will be served. This patch fixes this recalculating incorrect values.

lars18th · 2023-06-12T09:45:26Z

Hi @Jalle19 ,

Check it and merge.
Regards.

Jalle19 · 2023-06-12T10:00:27Z

src/minisatip.c

@@ -1939,7 +1948,8 @@ int main(int argc, char *argv[]) {
                          (socket_action)ssdp_discovery);
        if (si < 0 || si1 < 0)
            FAIL("sockets_add failed for ssdp");
-    }
+    } else
+        generate_uuid();


Is this needed? Would it be possible to call generate_uuid() just in one place?

Yes, it's necessary because the global var uuidi that controls the UUID generation. So we need to be sure that in all cases the UUID is created. So for reason and in order to not generate side effects errors if the code changes in the future, then it's preferable to call to the new generate_uuid() function every time a SSDP call is done and overcomed by the opts.disable_ssdp option. Therefore it's more safe to add this line here.

Please check it and merge! 😉

I still think it would be preferable to call generate_uuid() just once sometime during startup. Then we could even get rid of uuidi completely.

Hi @Jalle19 ,

That's my original idea. However, I'm not the one that has introduced the uuidi var. This var has specifically the objective of caching the creation of the UUID. I feel this was introduced (by @catalinii) to overcome potential errors of different edge cases or to permit to recalculate it. Anyway the fix is clean, simple and it works, so I suggest to merge it.

Regards.

Since the UUID is static for the duration of the program I think it's best to refactor this while we're at it so that the UUID is generated only once during startup. The uuidi variable was perhaps added to protect against the UUID suddenly changing if the MAC address changes (not very common but could theoretically happen).

You can generate the UUID somewhere in the startup logic, before the SSDP and HTTP sockets etc. are open.

Hi @Jalle19 ,

I undertand you suggestion. And I agree with the idea of some refactor. However, now I don't have much time. So because the code is clean and it fixes the bug, why not merge it now and in the future someone will refactor it?

I'll see if I can find the time to do the necessary changes

Hi @catalinii ,

Can you comment please?

A lot of things have been done historically for reasons probably they are no longer valid.
Cleaning the code is the best way forward IMHO, so I think that stripping the code out from there and putting it in the initialization part would make the code more clean and it would be called just once.

Hi @catalinii ,

Thank you for the response. I agree with you and @Jalle19 . I prefer it the code will be cleaned. However, I don't have time now to do it. So I hope @Jalle19 could enhance the PR. In fact it's sufficient to check all calls to generate_uuid(). The problem in my case it's the time necessary to do an extensible check of all edge cases. And for this reason I've suggested to merge the PR and after refactor the code. Perhaps the most simple way is to encapsulate all initialization tasks inside a new function and execute the call to generate_uuid() inside it.

Regards.

Jalle19 · 2024-03-22T13:51:38Z

Replaced by #1140

lars18th and others added 27 commits March 15, 2023 08:41

Handle CWs out of cycle (catalinii#1055)

066396b

* Handle CWs out of cycle * Change CW time after calculate expire time

Use bind address for the services configuration

62625a8

Use int for sorting pids, as each sid is 16 bit

58b58f1

Ignore EINTR when reading from socket catalinii#1074

44150f7

Revert default number of channels

e8f1a9f

Use unsigned int for copy32 and friends to avoid overflow in debug mode

cae8c47

Don't print empty channel names

c23dcc5

Add spaces between PIDs so the browser can break long lines

c78211c

Handle charset as unsigned

6bd1cc5

Add LICENSE file

73a5630

Fixes catalinii#1086

Reduce the size of the CAPMT to include only video and audio channels…

ad7a566

… and only CAIDs accepted by the CAM

Match CA if no CAID are added for dvbapi

f3bb91e

Don't build AXE builds with PMT=0

ad01e5c

Closes catalinii#1099

Remove non-working check (size is always 8)

479b724

Factor out "-o" parsing to parse_dvbapi_opt()

c35a895

Eventually all parameter parsing should be factored out to separate functions like this so they can be tested in isolation

Add tests for parse_dvbapi_opt(), fix two bugs in it

1246944

- specifying "~" made the hostname contain "~" if an offset was specified - specifying an offset truncated the hostname

Fix building with --enable-axe --disable-dvbca

b42da8f

Add a build step to verify that building for AXE works without libssl…

d968ac2

…-dev

Fix high cpu usage when CAM not plugged in for enigma catalinii#1096

b233de6

Fix warning in stream.c

297bd9f

Fix warning in pmt.c

e277737

Clear warning of unused code in pmt.c

cea601f

Clear warnings in utils.c

964dd4a

--static should create a static binary catalinii#1107

8d33e93

Fix UUID generation with SSDP disabled

9299c94

lars18th mentioned this pull request Jun 12, 2023

UUID not generated if SSDP discovery is disabled #1078

Closed

Jalle19 reviewed Jun 12, 2023

View reviewed changes

catalinii force-pushed the master branch from 89ffd78 to 80578a2 Compare September 6, 2023 19:11

Jalle19 mentioned this pull request Mar 22, 2024

Decouple UUID generation from SSDP discovery #1140

Merged

Jalle19 closed this Mar 22, 2024

lars18th deleted the fix-uuid branch March 22, 2024 16:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix UUID generation with SSDP disabled #1119

Fix UUID generation with SSDP disabled #1119

lars18th commented Jun 12, 2023

lars18th commented Jun 12, 2023

Jalle19 Jun 12, 2023

lars18th Jun 12, 2023

Jalle19 Jun 13, 2023 •

edited

Loading

lars18th Jun 13, 2023

Jalle19 Jun 13, 2023

lars18th Jun 14, 2023

Jalle19 Jun 14, 2023

lars18th Jun 19, 2023

catalinii Jun 19, 2023

lars18th Jun 19, 2023

Jalle19 commented Mar 22, 2024

Fix UUID generation with SSDP disabled #1119

Fix UUID generation with SSDP disabled #1119

Conversation

lars18th commented Jun 12, 2023

lars18th commented Jun 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jalle19 Jun 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jalle19 commented Mar 22, 2024

Jalle19 Jun 13, 2023 •

edited

Loading