Merge lp:~nickpapior/siesta/4.0-fdf into lp:siesta/4.0

Proposed by Nick Papior
Status: Merged
Approved by: Alberto Garcia
Approved revision: 544
Merged at revision: 544
Proposed branch: lp:~nickpapior/siesta/4.0-fdf
Merge into: lp:siesta/4.0
Diff against target: 108 lines (+28/-10)
3 files modified
Src/fdf/fdf.F90 (+4/-2)
Src/fdf/parse.F90 (+23/-7)
version.info (+1/-1)
To merge this branch: bzr merge lp:~nickpapior/siesta/4.0-fdf
Reviewer Review Type Date Requested Status
Alberto Garcia Approve
Review via email: mp+332964@code.launchpad.net

Description of the change

Fixed bug, however, I am mainly concerned whether anything will be broken by this.

I can't see why, but good to get a new view on this.

To post a comment you must log in.
Revision history for this message
Alberto Garcia (albertog) wrote :

I cannot see any downsides.

For the next iteration, we might make the 'line' component of the parsed-line object into
an allocatable character variable, and the token markers into allocatable arrays.

review: Approve
Revision history for this message
Nick Papior (nickpapior) wrote :

Exactly my thought, it does however require some restructuring due to array vs. string sub-indexing.

But would be nice to have!

I will merge.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Src/fdf/fdf.F90'
2--- Src/fdf/fdf.F90 2017-09-27 11:54:09 +0000
3+++ Src/fdf/fdf.F90 2017-10-29 13:48:37 +0000
4@@ -827,7 +827,8 @@
5 logical :: dump
6 logical, allocatable :: found(:)
7 character(80) :: msg
8- character(len=MAX_LENGTH) :: line, label, inc_file
9+ character(len=MAX_LENGTH) :: label, inc_file
10+ character(len=MAX_LENGTH*2):: line
11 integer(ip) :: i, ierr, ntok, ind_less, nlstart
12 type(parsed_line), pointer :: pline
13
14@@ -1044,7 +1045,8 @@
15 logical :: dump, found_elem
16 logical, pointer :: found_loc(:)
17 character(80) :: msg
18- character(len=MAX_LENGTH) :: line, inc_file, label
19+ character(len=MAX_LENGTH*2):: line
20+ character(len=MAX_LENGTH) :: label, inc_file
21 integer(ip) :: i, ierr, ntok, ind_less, nlstart
22 integer(ip) :: elem, nelem_loc
23 integer(ip), pointer :: found_index(:)
24
25=== modified file 'Src/fdf/parse.F90'
26--- Src/fdf/parse.F90 2013-12-11 14:01:58 +0000
27+++ Src/fdf/parse.F90 2017-10-29 13:48:37 +0000
28@@ -686,6 +686,7 @@
29 ! grab the seperator
30 sep = names(lpline,1,after=ti)
31 if ( leqi(sep,'to') .or. &
32+ leqi(sep,':') .or. &
33 leqi(sep,'--') .or. &
34 leqi(sep,'---') ) then
35
36@@ -1088,7 +1089,7 @@
37 SUBROUTINE parses(ntokens, line, first, last)
38 implicit none
39 !------------------------------------------------- Input Variables
40- character(len=MAX_LENGTH) :: line
41+ character(len=*) :: line
42
43 !------------------------------------------------ Output Variables
44 integer(ip) :: ntokens
45@@ -1097,7 +1098,7 @@
46 !------------------------------------------------- Local Variables
47 logical :: intoken, instring, completed
48 logical :: inlist
49- integer(ip) :: i, c, stringdel
50+ integer(ip) :: i, c, stringdel, length
51
52 ! Character statement functions
53 logical :: is_digit, is_upper, is_lower, is_alpha, &
54@@ -1138,9 +1139,12 @@
55 inlist = .FALSE.
56 stringdel = 0
57
58+ ! Trim space at the end (not from the left)
59+ length = len_trim(line)
60+
61 i = 1
62 completed = .FALSE.
63- do while((i .le. len(line)) .and. (.not. completed))
64+ do while((i <= length) .and. (.not. completed))
65 c = ichar(line(i:i))
66
67 ! Possible comment...
68@@ -1219,9 +1223,21 @@
69 endif
70
71 i = i + 1
72- enddo
73-
74- if (parse_debug) then
75+
76+ ! Check whether the parsing is correctly handled
77+ if ( i > MAX_LENGTH ) then
78+ ! Because we will limit search to the len_trim length,
79+ ! then this should only be found when the line has "content" too long.
80+ ! Note that this will *never* be executed if a comment is too
81+ ! long because it is checked as the first requirement and then
82+ ! completes parsing the line.
83+ call die('PARSE module: parses', 'Too long line (132 char): ' // &
84+ trim(line), THIS_FILE, __LINE__)
85+ end if
86+
87+ enddo
88+
89+ if (parse_debug) then
90 write(parse_log,*) 'PARSER:', ntokens, 'token(s)'
91 do i= 1, ntokens
92 write(parse_log,*) ' Token:', '|',line(first(i):last(i)),'|'
93@@ -1237,7 +1253,7 @@
94 SUBROUTINE morphol(ntokens, line, first, last, token_id)
95 implicit none
96 !------------------------------------------------- Input Variables
97- character(len=MAX_LENGTH) :: line
98+ character(len=*) :: line
99 integer(ip) :: ntokens
100 integer(ip) :: first(MAX_NTOKENS), last(MAX_NTOKENS)
101
102
103=== modified file 'version.info'
104--- version.info 2017-10-18 16:14:45 +0000
105+++ version.info 2017-10-29 13:48:37 +0000
106@@ -1,1 +1,1 @@
107-siesta-4.0--543
108+siesta-4.0--543--fdf-1

Subscribers

People subscribed via source and target branches