[PATCH] Don't use c++ keyword in public header files

[PATCH] Don't use c++ keyword in public header files

[Lucas De Marchi]

[-- Attachment #1: Type: text/plain, Size: 643 bytes --]

---
 include/sim.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/sim.h b/include/sim.h
index 81df60e..6e93769 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -154,7 +154,7 @@ struct ofono_sim_driver {
 			ofono_sim_lock_unlock_cb_t cb, void *data);
 	void (*change_passwd)(struct ofono_sim *sim,
 			enum ofono_sim_password_type type,
-			const char *old, const char *new,
+			const char *p_old, const char *p_new,
 			ofono_sim_lock_unlock_cb_t cb, void *data);
 	void (*lock)(struct ofono_sim *sim, enum ofono_sim_password_type type,
 			int enable, const char *passwd,
-- 
1.7.3.5

Re: [PATCH] Don't use c++ keyword in public header files

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 933 bytes --]

Hi Lucas,

>  include/sim.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/sim.h b/include/sim.h
> index 81df60e..6e93769 100644
> --- a/include/sim.h
> +++ b/include/sim.h
> @@ -154,7 +154,7 @@ struct ofono_sim_driver {
>  			ofono_sim_lock_unlock_cb_t cb, void *data);
>  	void (*change_passwd)(struct ofono_sim *sim,
>  			enum ofono_sim_password_type type,
> -			const char *old, const char *new,
> +			const char *p_old, const char *p_new,
>  			ofono_sim_lock_unlock_cb_t cb, void *data);
>  	void (*lock)(struct ofono_sim *sim, enum ofono_sim_password_type type,
>  			int enable, const char *passwd,

why is this exactly a problem?

And if you really wanna do that, then please not something cryptic like
p_new. Names like new_passwd or new_pw are way more descriptive.

Another possibility is using the combination current + passwd.

Regards

Marcel

Re: [PATCH] Don't use c++ keyword in public header files

[Lucas De Marchi]

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]

Hi Marcel

On Tue, Jan 25, 2011 at 9:54 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> -                     const char *old, const char *new,
>> +                     const char *p_old, const char *p_new,
>>                       ofono_sim_lock_unlock_cb_t cb, void *data);
>>       void (*lock)(struct ofono_sim *sim, enum ofono_sim_password_type type,
>>                       int enable, const char *passwd,
>
> why is this exactly a problem?

I did it only because the file contains this:

#ifdef __cplusplus
extern "C" {
#endif


So I thought someone might want to write a plugin in C++ or something
that includes sim.h

> And if you really wanna do that, then please not something cryptic like

No, I don't want. If nobody will use C++, I'm fine the way it is. Then
we might want to remove that ifdef, because it will not compile
anyway.



regards,

Lucas De Marchi

Re: [PATCH] Don't use c++ keyword in public header files

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

Hi Lucas,

> >> -                     const char *old, const char *new,
> >> +                     const char *p_old, const char *p_new,
> >>                       ofono_sim_lock_unlock_cb_t cb, void *data);
> >>       void (*lock)(struct ofono_sim *sim, enum ofono_sim_password_type type,
> >>                       int enable, const char *passwd,
> >
> > why is this exactly a problem?
> 
> I did it only because the file contains this:
> 
> #ifdef __cplusplus
> extern "C" {
> #endif
> 
> 
> So I thought someone might want to write a plugin in C++ or something
> that includes sim.h

and at that point you told the compiler that it is C code and not C++
and it should be just fine.

> > And if you really wanna do that, then please not something cryptic like
> 
> No, I don't want. If nobody will use C++, I'm fine the way it is. Then
> we might want to remove that ifdef, because it will not compile
> anyway.

I think it will compile just fine. Try to test it ;)

Regards

Marcel

Re: [PATCH] Don't use c++ keyword in public header files

[Lucas De Marchi]

[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]

On Tue, Jan 25, 2011 at 12:58 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Lucas,
>
>> >> -                     const char *old, const char *new,
>> >> +                     const char *p_old, const char *p_new,
>> >>                       ofono_sim_lock_unlock_cb_t cb, void *data);
>> >>       void (*lock)(struct ofono_sim *sim, enum ofono_sim_password_type type,
>> >>                       int enable, const char *passwd,
>> >
>> > why is this exactly a problem?
>>
>> I did it only because the file contains this:
>>
>> #ifdef __cplusplus
>> extern "C" {
>> #endif
>>
>>
>> So I thought someone might want to write a plugin in C++ or something
>> that includes sim.h
>
> and at that point you told the compiler that it is C code and not C++
> and it should be just fine.

This only works for the link phase, because of the name mangling in C++.

> I think it will compile just fine. Try to test it ;)

It doesn't. Example:

c.h
---
#ifdef __cplusplus
extern "C" {
#endif

int func(int *old, int *new);


#ifdef __cplusplus
}
#endif

---

cpp.cpp
---
#include <iostream>
#include "c.h"

using namespace std;


int main(int argc, char *argv[])
{
    int a = 2;
    int b = 3;


    cout << "Isso é um teste\n";

    return func(&a, &b);
}

---

$ g++ -c -o cpp.o cpp.cpp
In file included from cpp.cpp:2:0:
c.h:5:25: error: expected ‘,’ or ‘...’ before ‘new’



regards

Lucas De Marchi

Re: [PATCH] Don't use c++ keyword in public header files

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]

Hi Lucas,

> >> >> -                     const char *old, const char *new,
> >> >> +                     const char *p_old, const char *p_new,
> >> >>                       ofono_sim_lock_unlock_cb_t cb, void *data);
> >> >>       void (*lock)(struct ofono_sim *sim, enum ofono_sim_password_type type,
> >> >>                       int enable, const char *passwd,
> >> >
> >> > why is this exactly a problem?
> >>
> >> I did it only because the file contains this:
> >>
> >> #ifdef __cplusplus
> >> extern "C" {
> >> #endif
> >>
> >>
> >> So I thought someone might want to write a plugin in C++ or something
> >> that includes sim.h
> >
> > and at that point you told the compiler that it is C code and not C++
> > and it should be just fine.
> 
> This only works for the link phase, because of the name mangling in C++.
> 
> > I think it will compile just fine. Try to test it ;)
> 
> It doesn't. Example:

fair enough. Send a patch that uses one of the other proposed names. And
you might wanna update the atmodem and isimodem driver as well to just
be consistent.

Regards

Marcel

Re: [PATCH] Don't use c++ keyword in public header files

[Lucas De Marchi]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

Hi Marcel

On Wed, Jan 26, 2011 at 7:58 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> It doesn't. Example:
>
> fair enough. Send a patch that uses one of the other proposed names. And
> you might wanna update the atmodem and isimodem driver as well to just
> be consistent.
>

Only atmodem implements the change_passwd method:

$ grep -RIn change_passwd drivers/*
drivers/atmodem/sim.c:882:static void at_change_passwd(struct ofono_sim *sim,
drivers/atmodem/sim.c:1038:	.change_passwd		= at_change_passwd,


I'm going to change it too.


regards,

Lucas De Marchi