[OPW,kernel,v3,2/4] firmware: efi: Add timespec to timespec64 conversion
diff mbox

Message ID 1415123521-4564-3-git-send-email-somyaanand214@gmail.com
State New, archived
Headers show

Commit Message

Somya Anand Nov. 4, 2014, 5:51 p.m. UTC
32-bit systems using 'struct timespec' will break in the year 2038,
so we have to replace that code with more appropriate types.

Introducing a local variable of type 'struct timespec64' to update
interface of pstore_info.read and pstore_info.erase in order to
support new pstore_info API.

Signed-off-by: Somya Anand <somyaanand214@gmail.com>
---
Changes since version 2:
  * Use timespec64_to_timespec conversion for output function.
  * Reword commit message.
 drivers/firmware/efi/efi-pstore.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

Comments

Arnd Bergmann Nov. 4, 2014, 9:37 p.m. UTC | #1
On Tuesday 04 November 2014 23:21:59 Somya Anand wrote:
> @@ -51,7 +51,8 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
>  	int i;
>  	int cnt;
>  	unsigned int part;
> -	unsigned long time, size;
> +	unsigned long size;
> +	u64 time;
>  
>  	if (efi_guidcmp(entry->var.VendorGuid, vendor))
>  		return 0;
> @@ -59,7 +60,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
>  	for (i = 0; i < DUMP_NAME_LEN; i++)
>  		name[i] = entry->var.VariableName[i];
>  
> -	if (sscanf(name, "dump-type%u-%u-%d-%lu-%c",
> +	if (sscanf(name, "dump-type%u-%u-%d-%llu-%c",
>  		   cb_data->type, &part, &cnt, &time, &data_type) == 5) {
>  		*cb_data->id = generic_id(time, part, cnt);
>  		*cb_data->count = cnt;
> @@ -69,14 +70,14 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
>  			*cb_data->compressed = true;
>  		else
>  			*cb_data->compressed = false;
> -	} else if (sscanf(name, "dump-type%u-%u-%d-%lu",
> +	} else if (sscanf(name, "dump-type%u-%u-%d-%llu",
>  		   cb_data->type, &part, &cnt, &time) == 4) {
>  		*cb_data->id = generic_id(time, part, cnt);
>  		*cb_data->count = cnt;
>  		cb_data->timespec->tv_sec = time;
>  		cb_data->timespec->tv_nsec = 0;
>  		*cb_data->compressed = false;
> -	} else if (sscanf(name, "dump-type%u-%u-%lu",
> +	} else if (sscanf(name, "dump-type%u-%u-%llu",
>  			  cb_data->type, &part, &time) == 3) {
>  		/*
>  		 * Check if an old format,

This part looks good


> @@ -232,6 +235,8 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
>  	efivar_entry_iter_end();
>  	if (size <= 0)
>  		kfree(*data.buf);
> +	ts_temp = timespec64_to_timespec(time);
> +	ts = &ts_temp;
>  	return size;
>  }
>  

Here you have made the same mistake as in the first patch, also seems
to be present in the other two.

> @@ -292,8 +297,8 @@ static int efi_pstore_erase_func(struct efivar_entry *entry, void *data)
>  		 * Check if an old format, which doesn't support
>  		 * holding multiple logs, remains.
>  		 */
> -		sprintf(name_old, "dump-type%u-%u-%lu", ed->type,
> -			(unsigned int)ed->id, ed->time.tv_sec);
> +		sprintf(name_old, "dump-type%u-%u-%ld", ed->type,
> +			(unsigned int)ed->id, ed->timespec.tv_sec);
>  
>  		for (i = 0; i < DUMP_NAME_LEN; i++)
>  			efi_name_old[i] = name_old[i];

This is still broken on 32-bit builds. Did you compile it for both
32 and 64 bit? The compiler should have warned about it.

	Arnd

Patch
diff mbox

diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index e992abc..e2ac1ce 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -32,7 +32,7 @@  struct pstore_read_data {
 	u64 *id;
 	enum pstore_type_id *type;
 	int *count;
-	struct timespec *timespec;
+	struct timespec64 *timespec;
 	bool *compressed;
 	char **buf;
 };
@@ -51,7 +51,8 @@  static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
 	int i;
 	int cnt;
 	unsigned int part;
-	unsigned long time, size;
+	unsigned long size;
+	u64 time;
 
 	if (efi_guidcmp(entry->var.VendorGuid, vendor))
 		return 0;
@@ -59,7 +60,7 @@  static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
 	for (i = 0; i < DUMP_NAME_LEN; i++)
 		name[i] = entry->var.VariableName[i];
 
-	if (sscanf(name, "dump-type%u-%u-%d-%lu-%c",
+	if (sscanf(name, "dump-type%u-%u-%d-%llu-%c",
 		   cb_data->type, &part, &cnt, &time, &data_type) == 5) {
 		*cb_data->id = generic_id(time, part, cnt);
 		*cb_data->count = cnt;
@@ -69,14 +70,14 @@  static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
 			*cb_data->compressed = true;
 		else
 			*cb_data->compressed = false;
-	} else if (sscanf(name, "dump-type%u-%u-%d-%lu",
+	} else if (sscanf(name, "dump-type%u-%u-%d-%llu",
 		   cb_data->type, &part, &cnt, &time) == 4) {
 		*cb_data->id = generic_id(time, part, cnt);
 		*cb_data->count = cnt;
 		cb_data->timespec->tv_sec = time;
 		cb_data->timespec->tv_nsec = 0;
 		*cb_data->compressed = false;
-	} else if (sscanf(name, "dump-type%u-%u-%lu",
+	} else if (sscanf(name, "dump-type%u-%u-%llu",
 			  cb_data->type, &part, &time) == 3) {
 		/*
 		 * Check if an old format,
@@ -208,17 +209,19 @@  static int efi_pstore_sysfs_entry_iter(void *data, struct efivar_entry **pos)
  *           and pstore will stop reading entry.
  */
 static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
-			       int *count, struct timespec *timespec,
+			       int *count, struct timespec *ts,
 			       char **buf, bool *compressed,
 			       struct pstore_info *psi)
 {
+	struct timespec64 time;
+	struct timespec ts_temp;
 	struct pstore_read_data data;
 	ssize_t size;
 
 	data.id = id;
 	data.type = type;
 	data.count = count;
-	data.timespec = timespec;
+	data.timespec = &time;
 	data.compressed = compressed;
 	data.buf = buf;
 
@@ -232,6 +235,8 @@  static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
 	efivar_entry_iter_end();
 	if (size <= 0)
 		kfree(*data.buf);
+	ts_temp = timespec64_to_timespec(time);
+	ts = &ts_temp;
 	return size;
 }
 
@@ -266,7 +271,7 @@  struct pstore_erase_data {
 	u64 id;
 	enum pstore_type_id type;
 	int count;
-	struct timespec time;
+	struct timespec64 timespec;
 	efi_char16_t *name;
 };
 
@@ -292,8 +297,8 @@  static int efi_pstore_erase_func(struct efivar_entry *entry, void *data)
 		 * Check if an old format, which doesn't support
 		 * holding multiple logs, remains.
 		 */
-		sprintf(name_old, "dump-type%u-%u-%lu", ed->type,
-			(unsigned int)ed->id, ed->time.tv_sec);
+		sprintf(name_old, "dump-type%u-%u-%ld", ed->type,
+			(unsigned int)ed->id, ed->timespec.tv_sec);
 
 		for (i = 0; i < DUMP_NAME_LEN; i++)
 			efi_name_old[i] = name_old[i];
@@ -319,8 +324,9 @@  static int efi_pstore_erase_func(struct efivar_entry *entry, void *data)
 }
 
 static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count,
-			    struct timespec time, struct pstore_info *psi)
+			    struct timespec ts, struct pstore_info *psi)
 {
+	struct timespec64 time = timespec_to_timespec64(ts);
 	struct pstore_erase_data edata;
 	struct efivar_entry *entry = NULL;
 	char name[DUMP_NAME_LEN];
@@ -338,7 +344,7 @@  static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count,
 	edata.id = part;
 	edata.type = type;
 	edata.count = count;
-	edata.time = time;
+	edata.timespec = time;
 	edata.name = efi_name;
 
 	efivar_entry_iter_begin();