net.websocket: swap unsafe use of nil for a safe default value

[AlfieRan]

Removed a use of unsafe { nil } from the net.websocket module due to it causing edge case errors where the V compiler generated C code that tried to use a non-existent function that converted an integer to the log.Logger interface.

The value I replaced this with (in both cases as it was effectively the same code) was &log.Logger(&log.Log{level: .info}) since that is the default value supplied by the ServerOpt and ClientOpt structs when the initiate the struct of which was edited.

Here's the ServerOpt struct for proof: [params] pub struct ServerOpt { logger &log.Logger = &log.Logger(&log.Log{ level: .info }) }

[shove70]

Sorry, there was a slight change a few days ago, and a small rule was added: the reference type field of the structure must be initialized. Therefore, we initialize uniformly and explicitly, and the initial value is nil, Sorry for the inconvenience this change has brought to you. However, in order to better V, it is worth our efforts.

In fact, both net/websocket/client and net/websocket/server provide a way to build them:

server := websocket.new_server(addr.family(), aport, '', ServerOpt{})

Source code:

[params]
pub struct ServerOpt {
	logger &log.Logger = &log.Logger(&log.Log{
	level: .info
})
}

// new_server instance a new websocket server on provided port and route
pub fn new_server(family net.AddrFamily, port int, route string, opt ServerOpt) &Server {
	return &Server{
		ls: 0
		family: family
		port: port
		logger: opt.logger
		state: .closed
	}
}

I think the best way is to perform actual initialization when necessary, such as server_ test.v, client_ test.v Like the examples, they all work well.

new_client(), new_server() returns the reference type, which avoids the unexpected copy and incomprehensibility caused by the inadvertent use of non reference types.

In some languages, some structures are prohibited from being initialized outside the package.

[shove70]

If the initialization of the structure in the module is a little complicated or involves memory optimization, the authors will generally provide new_xxx() to guide users how to use it. On the contrary, some very simple structures may ignore this step.

[AlfieRan]

Thanks for your response,

That does indeed work, most of the time, however in some very specific scenarios - most of which seem to occur when the server/client is a property within a struct that is then being passed into a function/another struct - the compiler is not initialising the base server/client struct as would be expected and then creates C could in which the program is trying to convert “voidptr” to meet the interface “log.Logger” through the use of a C function named ‘I_voidptr_to_Interface_log__Logger‘ which doesn’t actually exist.

This is what generated the error that lead me down this rabbit hole to begin with: ‘error: implicit declaration of function 'I_voidptr_to_Interface_log__Logger' is invalid in C99 [-Werror,-Wimplicit-function-declaration]’

Although this may not be an issue specific to the web socket module making it a more general issue or could also be due to how I’m handling the structs of which the server and client structs are a part of which would make it more of a me issue, it still seems like weird behaviour that might be worth looking into if anyone who reads this has the time and energy to do so.

tldr - extreme cases with layers upon layers of structs cause issues with this method.

[shove70]

You're welcome.

I have no problem:

import net
import net.websocket

fn main() {
	addr := net.resolve_ipaddrs('127.0.0.1', .ip, net.SocketType.tcp)?[0]
	mut server := websocket.new_server(addr.family(), 8080, '', websocket.ServerOpt{})
	server.free()
}

Can you post a short piece of code like this? thank you

[shove70]

If the problem really exists, I personally suggest that this PR turn to optimize the Log module. for reference only :)

[shove70]

import net
import net.websocket

struct AnotherStruct {
mut:
	server &websocket.Server = unsafe { nil }
}

fn new_another_struct() ?&AnotherStruct {
	addr := net.resolve_ipaddrs('127.0.0.1', .ip, net.SocketType.tcp)?[0]
	another_struct := AnotherStruct {
		server: websocket.new_server(addr.family(), 8080, '', websocket.ServerOpt{})
	}
	return &another_struct
}

fn main() {
	mut another_struct := new_another_struct()?
	another_struct.server.free()
}

It also works.

[spytheman]

Needs v fmt -w vlib/net/websocket/*.v to pass the CI.

[spytheman]

It needs a small test too. I intend to use the example program in https://github.com/vlang/v/issues/15839 , unless someone has a smaller one?

[spytheman]

Note that this PR only fixes the codegen for net.websocket, it does not solve the original problem. I'll file a separate issue or add to https://github.com/vlang/v/issues/15839, when I manage to distill it to a smaller case.

[spytheman]

Please leave it as draft. I left it intentionally as draft, and I'll mark it as ready for review and merge it, when all the CI passes. I am still fixing it (some of the CI jobs do not have openssl installed, and the net.websocket module imports net.openssl, i.e. the tests that import it, have to be skipped on these CI jobs).

[AlfieRan]

Sorry! I was reading the responses on my phone and must've accidentally switched it to ready to review by accident, I did not mean to do that.

[spytheman]

Sorry! I was reading the responses on my phone and must've accidentally switched it to ready to review by accident, I did not mean to do that.

No problem. I should have explained why I put it as draft when I did it.