Skip to content
This repository has been archived by the owner on Dec 1, 2020. It is now read-only.

No header definitions for exposed functions in uv_unix_udp.c #49

Open
nkolban opened this issue Nov 10, 2016 · 12 comments
Open

No header definitions for exposed functions in uv_unix_udp.c #49

nkolban opened this issue Nov 10, 2016 · 12 comments

Comments

@nkolban
Copy link

nkolban commented Nov 10, 2016

This source file contains exposed functions which appear to be used elsewhere ... uv_unix_udp.c

However there is no header definition for the exposed functions.

@chokobole
Copy link

@nkolban I exposed externally used function in include/uv__udp.h. Which functions do you think I missed?

@nkolban
Copy link
Author

nkolban commented Nov 10, 2016

In the source file source/unix/uv_unix_udp.c here is (for example) a function called uv__udp_send(). This function appears to be used in source/uv_udp.c. However since there appears to be no definition of uv__udp_send in an headers .... it appears to be missing.

Also ... and this is a question ... aren't files in souce/unix/* platform specific and hence definitions shouldn't be found in generic code such as source/uv_udp.c?

@chokobole
Copy link

chokobole commented Nov 10, 2016

@nkolban Because external project using libtuv we want to use uv_udp_send not uv__udp_send. Second, yes, unix/* is implemented platfrom specifically. in order to remove redundancy, duplicable or platform common codes reside in uv_udp.c . Platform specific code will be diversed with uv__udp_... in source/unix , source/nuttx or so on.

@nkolban
Copy link
Author

nkolban commented Nov 10, 2016

Many thanks for the quick responses. If I am understanding correctly, functions such as uv__udp_... are meant to be platform specific. Does that mean that there should be a header file which defines the expected signatures of functions that are expected to be implemented by a platform? If that is the case, I think we are missing a header file for uv__udp...* ... or if not a header file, then forward declarations for them before use in the C source files that use them.

@chokobole
Copy link

@nkolban Yes, you're right. uv__udp_... is platform specific. But I don't understand why we need header file or forward declaration. If you are worried about compilation, it's safely compiled. Because we didn't put static access modifier, it's visible to current compile unit? Maybe do you want to use those functions such as uv__udp_ outside?

@nkolban
Copy link
Author

nkolban commented Nov 10, 2016

I'm compiling source/uv_udp.c. I'm compiling using a GNU gcc cross compiler on Linux generating ESP32 binary. When I compile source/uv_udp.c the compilation fails with:

/home/kolban/esp32/esptest/apps/workspace/libtvu/components/libtuv/source/uv_udp.c: In function 'uv_udp_send':
/home/kolban/esp32/esptest/apps/workspace/libtvu/components/libtuv/source/uv_udp.c:75:3: error: implicit declaration of function 'uv__udp_send' [-Werror=implicit-function-declaration]
   return uv__udp_send(req, handle, bufs, nbufs, addr, addrlen, send_cb);

The reason appears to be that at line 75 of uv_udp.c there is a call to uv__udp_send but it has not been pre-declared ... and hence the compiler can't validate that there is indeed a uv__udp_send() function and that the parameters match its signature.

@chokobole
Copy link

I believe the reason is that you didn't compile with source/unix/*.c together? Could you tell me how you compiled?

@nkolban
Copy link
Author

nkolban commented Nov 10, 2016

Howdy ... it isn't a "linkage" error ... a linkage error would say that I have source file A.c which declared a function called a() and source file B.c wanted to use function a() then I would have to link A.o and B.o together. That would be a linkage error.

However if in source file B.c I want to use function a() then in the source code of B.c I would need a reference declaration.

For example:

A.c

void a(int x, int y, int z) {
  // Implementation of a() code
}

and in B.c I would need

void a(int x, int y, int z); /// This is the forward declaration of a()

void b() {
   a(1,2,3);
}

Typically this is done by having a header file such as:
A.h

void a(int x, int y, int z); /// This is the forward declaration of a()

and then B.c would become:

#include "A.h"

void b() {
   a(1,2,3);
}

@chokobole
Copy link

Ah, I found this. let's assume that we have a.c, b.c and main.c like below as you said.

// a.c
#include <stdio.h>

void a() {
  printf("a.c\n");
}
// b.c
int b() {
  a();
}
// main.c
int main() {
  b();
}

compile with gcc is fine.

chokobole@wonyongkimPC:~/Workspace/iotjs/nuttx-iotjs-master/iotjs$ gcc -o main a.c b.c main.c
chokobole@wonyongkimPC:~/Workspace/iotjs/nuttx-iotjs-master/iotjs$ ./main 
a.c

this is because gcc allows implicit declaration of function in default. So when you compiling with -Wall, then it annoies with warning.

chokobole@wonyongkimPC:~/Workspace/iotjs/nuttx-iotjs-master/iotjs$ gcc -o main a.c b.c main.c -Wall
b.c: In function ‘b’:
b.c:2:3: warning: implicit declaration of function ‘a’ [-Wimplicit-function-declaration]
   a();
   ^
b.c:3:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
main.c: In function ‘main’:
main.c:2:3: warning: implicit declaration of function ‘b’ [-Wimplicit-function-declaration]
   b();
   ^
main.c:3:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^

However, compile with g++ is problematic.

chokobole@wonyongkimPC:~/Workspace/iotjs/nuttx-iotjs-master/iotjs$ g++ -o main a.c b.c main.c
b.c: In function ‘int b()’:
b.c:2:5: error: ‘a’ was not declared in this scope
   a();
     ^
main.c: In function ‘int main()’:
main.c:2:5: error: ‘b’ was not declared in this scope
   b();
     ^

In conclusion, I think I admit I was wrong, and we need header file to declare functions in header file or use forward declaration.

@nkolban
Copy link
Author

nkolban commented Nov 10, 2016

Howdy my friend ... no wrong or right ... we are all friends here and I'm incorrect more than I'm on to something. Can I now assume that this issue is valid? If so I'll hold off until there is a fix.

@chokobole
Copy link

Thanks for the good points! Yes, we will fix this issue.

@glistening
Copy link

Yes, it is a bug. I belive we need an internal header that declares uv__udp_send. uv__udp.h would be a candidate since double underscore is used for internal purpose. However the file is already in use for declaring external udp APIs. We may use uv___udp.h.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants