libtuv icon indicating copy to clipboard operation
libtuv copied to clipboard

No header definitions for exposed functions in uv_unix_udp.c

Open nkolban opened this issue 9 years ago • 12 comments

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.

nkolban avatar Nov 10 '16 00:11 nkolban

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

chokobole avatar Nov 10 '16 01:11 chokobole

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?

nkolban avatar Nov 10 '16 01:11 nkolban

@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.

chokobole avatar Nov 10 '16 01:11 chokobole

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.

nkolban avatar Nov 10 '16 01:11 nkolban

@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?

chokobole avatar Nov 10 '16 02:11 chokobole

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.

nkolban avatar Nov 10 '16 02:11 nkolban

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

chokobole avatar Nov 10 '16 03:11 chokobole

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);
}

nkolban avatar Nov 10 '16 04:11 nkolban

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.

chokobole avatar Nov 10 '16 04:11 chokobole

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.

nkolban avatar Nov 10 '16 05:11 nkolban

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

chokobole avatar Nov 10 '16 05:11 chokobole

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.

glistening avatar Nov 10 '16 05:11 glistening